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

[Marcus Karlsson]

On Tue, Oct 16, 2012 at 05:03:37PM -0400, Cole Robinson wrote:
> 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.

OK. Got it.

Marcus