[virt-tools-list] [PATCH] Edit description immediately without a shutdown

[Cole Robinson]

On 10/16/2012 04:47 PM, Marcus Karlsson wrote:
> On Sun, Oct 14, 2012 at 03:26:26PM -0400, Cole Robinson wrote:
>> On 10/11/2012 05:40 PM, Marcus Karlsson wrote:
>>> Editing the description of a running guest will show a dialog that
>>> changes will take effect after the next guest shutdown. However, libvirt
>>> can change the description of a guest while it is running.
>>>
>>> Teach virt-manager to edit the description of a running guest without
>>> requiring a shutdown.
>>
>> Hi Marcus, thanks for the patch!
>>
>> However there's kind of a problem here, in that this will error for libvirt
>> less than 0.9.10 when setMetadata was introduced.
> 
> I see. That should of course be avoided. No need to break backwards
> compatibility.
> 
>> In current virt-manager though we already emulate description 'hotplug': we
>> always read the description from the inactive XML for the running VM, so a
>> 'define' basically works like a hotplug.
>>
>> The only missing piece was working around that error message, which I've done
>> with a new commit:
>>
>> http://git.fedorahosted.org/cgit/virt-manager.git/commit/?id=a341ce4534f60f79113ce27e64416abebcf241dd
>>
>> Also, I added a check to the virtinst support module to check for setMetadata
>> support:
>>
>> http://git.fedorahosted.org/cgit/python-virtinst.git/commit/?id=849c1e02a3939a4b9d57e62c4974e34526249f0e
>>
>> So if you want to update your patch, we could use setMetadata only when
>> supported. Move my comment from the virt-manager commit into the new
>> vmmDomain.description_hotplug function, and if setMetadata isn't supported,
>> just exit from the function early.
> 
> OK. I'll prepare a new patch.
> 
>> That way the only description hotplug hack can be used as a fallback.
> 
> This last sentence I don't understand. Do you mean that you want to keep
> the lambda as a fallback? Is it not enough to do just something like
> this in hotplug_description?
> 
> def hotplug_description(self, desc):
>     if virtinst.support.check_domain_support(self._backend,
>             virtinst.support.SUPPORT_DOMAIN_SET_METADATA):
>         flags = (libvirt.VIR_DOMAIN_AFFECT_LIVE |
>                 libvirt.VIR_DOMAIN_AFFECT_CONFIG)
>         self._backend.setMetadata(
>                 libvirt.VIR_DOMAIN_METADATA_DESCRIPTION,
>                 desc, None, None, flags)
> 
> That way setMetadata is not called if it's not supported. But
> hotplug_description is always called so the error message should not
> appear.
> 

Sorry for unclarity, that code is what I mean. Though stylistically I would
return early if that support condition is not met, so the setMetadata call
doesn't need to be indented.

Thanks,
Cole