Re: [libvirt] [PATCH 2/2] qemu: Emit compatible XML when migrating a domain
On Fri, May 04, 2012 at 17:28:39 -0600, Eric Blake wrote:
On 05/04/2012 04:40 PM, Jiri Denemark wrote:
> When we added the default USB controller into domain XML, we efficiently
> broke migration to older versions of libvirt that didn't support USB
> controllers at all (0.9.4 and earlier) even for domains that doesn't use
s/doesn't/don't/
> anything that the older libvirt can't provide. We still want to present
> the default USB controller in any XML seen by a user/app but we can
> safely remove it from the domain XML used during migration. If we are
> migrating to a new enough libvirt, it will add the controller XML back,
> while older libvirt won't be confused with it although it will still
> tell qemu to create the controller.
>
> Similar approach can be used in the future whenever we find out we
> always enabled some kind of device without properly advertising it in
> domain XML.
Memballoon would be such a device, if we care about migration to even
older libvirt.
Right.
> ---
> src/qemu/qemu_domain.c | 60 ++++++++++++++++++++++++++++++++++++++++----
> src/qemu/qemu_domain.h | 10 +++++--
> src/qemu/qemu_driver.c | 21 +++++++++------
> src/qemu/qemu_migration.c | 10 ++++---
> src/qemu/qemu_process.c | 8 +++---
> 5 files changed, 83 insertions(+), 26 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index b105644..3752ddf 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -1227,11 +1227,14 @@ int
> qemuDomainDefFormatBuf(struct qemud_driver *driver,
> virDomainDefPtr def,
> unsigned int flags,
> + bool compatible,
> virBuffer *buf)
Rather than adding a new 'compatible' flag and having to adjust _every_
caller, could we instead add a new VIR_DOMAIN_XML_INTERNAL flag to be
or'd in to flags only for the callers that care? On the other hand,
we'd have to filter that flag back out before calling into domain_conf,
so I guess this approach is as good as any.
Yeah, we would need to filter it out and we would need to either move the
internal flags enum to a more public place (it's internal to domain_conf.c
now) or make another enum and make sure neither of them conflicts with the
real flags enum :-) I decided to introduce new bool parameter since the
compiler checked are places I need to change for me and it's more explicit so
that any new caller of this api needs to explicitly think about the value to
pass for compatibility.
> + if (usb && usb->idx == 0 &&
usb->model == -1) {
Just like memballoon, if we ever want to explicitly represent an XML
with no usb controller, we will have to provide <controller type='usb'
mode='none'>; but there, the model would not be -1, so this would
properly be migrated.
Right and we already have a request to do just this. BTW, I don't explicitly
check the PCI address assigned to the controller because the default
automatically added USB1 controller must always have PCI address 0:0:1.2. When
a domains is configured with different address assigned to the default USB
controller, libvirt will refuse to start it.
ACK.
Thanks. I pushed this series.
Jirka