Playing directly with our live definition, updating it, and reverting it back once we are done is very nice and it's quite dangerous too. Let's just make a copy of the domain definition if needed and do all tricks on the copy. https://bugzilla.redhat.com/show_bug.cgi?id=1320470 Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/conf/domain_conf.c | 10 ++++++---- src/qemu/qemu_domain.c | 44 ++++++++++++++++++++++++-------------------- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index cf5eb1d..72ff5c1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -21039,10 +21039,12 @@ virDomainChrSourceDefFormat(virBufferPtr buf, break; case VIR_DOMAIN_CHR_TYPE_UNIX: - virBufferAsprintf(buf, "<source mode='%s'", - def->data.nix.listen ? "bind" : "connect"); - virBufferEscapeString(buf, " path='%s'", def->data.nix.path); - virDomainSourceDefFormatSeclabel(buf, nseclabels, seclabels, flags); + if (def->data.nix.path) { + virBufferAsprintf(buf, "<source mode='%s'", + def->data.nix.listen ? "bind" : "connect"); + virBufferEscapeString(buf, " path='%s'", def->data.nix.path); + virDomainSourceDefFormatSeclabel(buf, nseclabels, seclabels, flags); + } break; case VIR_DOMAIN_CHR_TYPE_SPICEPORT: diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 930e0b7..1b0279b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3071,19 +3071,25 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver, virBuffer *buf) { int ret = -1; - virCPUDefPtr cpu = NULL; - virCPUDefPtr def_cpu = def->cpu; - virDomainControllerDefPtr *controllers = NULL; - int ncontrollers = 0; + virDomainDefPtr copy = NULL; virCapsPtr caps = NULL; if (!(caps = virQEMUDriverGetCapabilities(driver, false))) goto cleanup; + if (!(flags & (VIR_DOMAIN_XML_UPDATE_CPU | VIR_DOMAIN_XML_MIGRATABLE))) + goto format; + + if (!(copy = virDomainDefCopy(def, caps, driver->xmlopt, + flags & VIR_DOMAIN_XML_MIGRATABLE))) + goto cleanup; + + def = copy; + /* Update guest CPU requirements according to host CPU */ if ((flags & VIR_DOMAIN_XML_UPDATE_CPU) && - def_cpu && - (def_cpu->mode != VIR_CPU_MODE_CUSTOM || def_cpu->model)) { + def->cpu && + (def->cpu->mode != VIR_CPU_MODE_CUSTOM || def->cpu->model)) { if (!caps->host.cpu || !caps->host.cpu->model) { virReportError(VIR_ERR_OPERATION_FAILED, @@ -3091,10 +3097,8 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver, goto cleanup; } - if (!(cpu = virCPUDefCopy(def_cpu)) || - cpuUpdate(cpu, caps->host.cpu) < 0) + if (cpuUpdate(def->cpu, caps->host.cpu) < 0) goto cleanup; - def->cpu = cpu; } if ((flags & VIR_DOMAIN_XML_MIGRATABLE)) { @@ -3151,10 +3155,11 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver, } if (toremove) { - controllers = def->controllers; - ncontrollers = def->ncontrollers; + virDomainControllerDefPtr *controllers = def->controllers; + int ncontrollers = def->ncontrollers; + if (VIR_ALLOC_N(def->controllers, ncontrollers - toremove) < 0) { - controllers = NULL; + def->controllers = controllers; goto cleanup; } @@ -3163,23 +3168,22 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver, if (controllers[i] != usb && controllers[i] != pci) def->controllers[def->ncontrollers++] = controllers[i]; } + + VIR_FREE(controllers); + virDomainControllerDefFree(pci); + virDomainControllerDefFree(usb); } } - ret = virDomainDefFormatInternal(def, driver->caps, + format: + ret = virDomainDefFormatInternal(def, caps, virDomainDefFormatConvertXMLFlags(flags), buf); cleanup: - def->cpu = def_cpu; - virCPUDefFree(cpu); - if (controllers) { - VIR_FREE(def->controllers); - def->controllers = controllers; - def->ncontrollers = ncontrollers; - } + virDomainDefFree(copy); virObjectUnref(caps); return ret; } -- 2.9.0

Migration to an older libvirt (pre v1.3.0-175-g7140807) is broken because older versions of libvirt generated different channel paths and they didn't drop the default paths when parsing domain XMLs. We'd get such a nice error message: internal error: process exited while connecting to monitor: 2016-07-08T15:28:02.665706Z qemu-kvm: -chardev socket, id=charchannel0,path=/var/lib/libvirt/qemu/channel/target/ domain-3-nest/org.qemu.guest_agent.0,server,nowait: Failed to bind socket to /var/lib/libvirt/qemu/channel/target/domain-3-nest/ org.qemu.guest_agent.0: No such file or directory That said, we should not even format the default paths when generating a migratable XML. https://bugzilla.redhat.com/show_bug.cgi?id=1320470 Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_domain.c | 43 +++++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 1b0279b..0ef593c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2264,6 +2264,29 @@ qemuDomainDefaultNetModel(const virDomainDef *def, return "rtl8139"; } + +/* + * Clear auto generated unix socket path, i.e., the one which starts with our + * channel directory. + */ +static void +qemuDomainChrDefDropDefaultPath(virDomainChrDefPtr chr, + virQEMUDriverPtr driver) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + + if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL && + chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO && + chr->source.type == VIR_DOMAIN_CHR_TYPE_UNIX && + chr->source.data.nix.path && + STRPREFIX(chr->source.data.nix.path, cfg->channelTargetDir)) { + VIR_FREE(chr->source.data.nix.path); + } + + virObjectUnref(cfg); +} + + static int qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, const virDomainDef *def, @@ -2345,21 +2368,8 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, /* clear auto generated unix socket path for inactive definitions */ if ((parseFlags & VIR_DOMAIN_DEF_PARSE_INACTIVE) && - dev->type == VIR_DOMAIN_DEVICE_CHR && - dev->data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL && - dev->data.chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO && - dev->data.chr->source.type == VIR_DOMAIN_CHR_TYPE_UNIX && - dev->data.chr->source.data.nix.path && - STRPREFIX(dev->data.chr->source.data.nix.path, cfg->channelTargetDir)) { - /* - * If the address is generated by us (starts with our - * channel dir), we should not keep it in the persistent - * XML. If libvirt is the one who generated it, users - * shouldn't care about that. If they do, they are - * supposed to set it themselves. - */ - VIR_FREE(dev->data.chr->source.data.nix.path); - } + dev->type == VIR_DOMAIN_DEVICE_CHR) + qemuDomainChrDefDropDefaultPath(dev->data.chr, driver); /* forbid capabilities mode hostdev in this kind of hypervisor */ if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV && @@ -3174,7 +3184,8 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver, virDomainControllerDefFree(usb); } - + for (i = 0; i < def->nchannels; i++) + qemuDomainChrDefDropDefaultPath(def->channels[i], driver); } format: -- 2.9.0