[libvirt] [PATCH] qemu: clear useless code
The variable 'now' is useless in qemuMigrationPrepareAny(), so I clear it. Signed-off-by: Wang Yufei Signed-off-by: Zhang Bo --- src/qemu/qemu_migration.c | 4 1 file changed, 4 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index c1af704..4dcba7a 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -2913,7 +2913,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, int ret = -1; int dataFD[2] = { -1, -1 }; qemuDomainObjPrivatePtr priv = NULL; -unsigned long long now; qemuMigrationCookiePtr mig = NULL; bool tunnel = !!st; char *xmlout = NULL; @@ -2923,9 +2922,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, bool abort_on_error = !!(flags & VIR_MIGRATE_ABORT_ON_ERROR); bool taint_hook = false; -if (virTimeMillisNow(&now) < 0) -return -1; - virNWFilterReadLockFilterUpdates(); if (flags & VIR_MIGRATE_OFFLINE) { -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix double free when fail to cold-plug a rng device
On 05/12/2015 11:14 PM, Michal Privoznik wrote: On 12.05.2015 15:55, Luyao Huang wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1220809 When cold-plug a rng device and get failed in qemuDomainAssignAddresses, we will double free the rng device. Free the pointer after we Insert the device success to fix this issue. ... 5 0x7fb7d180ac8a in virFree at util/viralloc.c:582 6 0x7fb7d1895cdd in virDomainRNGDefFree at conf/domain_conf.c:19786 7 0x7fb7d1895d99 in virDomainDeviceDefFree at conf/domain_conf.c:2022 8 0x7fb7b92b8baf in qemuDomainAttachDeviceFlags at qemu/qemu_driver.c:8785 9 0x7fb7d190c5d7 in virDomainAttachDeviceFlags at libvirt-domain.c:8488 10 0x7fb7d23af9d2 in remoteDispatchDomainAttachDeviceFlags at remote_dispatch.h:2842 ... Signed-off-by: Luyao Huang --- src/qemu/qemu_driver.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 33c1cfd..f922a28 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8359,11 +8359,10 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, if (virDomainRNGInsert(vmdef, dev->data.rng, false) < 0) return -1; +dev->data.rng = NULL; if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) return -1; - -dev->data.rng = NULL; break; case VIR_DOMAIN_DEVICE_MEMORY: I've reworded the commit message a bit, ACKed and pushed. Thanks for quick review. Michal Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] conf: fix no error when set an unsupport string in ./devices/shmem/msi[@ioeventfd]
On 05/11/2015 10:08 PM, Martin Kletzander wrote: On Mon, May 11, 2015 at 08:59:37PM +0800, Luyao Huang wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1220265 Pass the return value to an enum directly is not safe. When pass a invalid @ioeventfd and virTristateSwitchTypeFromString return -1 to def->msi.ioeventfd, and this value transform to 4294967295, so no error when the parse failed. To fix this issue, folter the value using int and then assign it to virTristateSwitch as it's done. Signed-off-by: Luyao Huang --- v2: a new way to fix this issue. ACK, I reworded the commit message and pushed and I believe there is no need for a unit test. Yes, thanks for your quick review. I wonder why we don't rework out FromString() helpers to parse the value into a parameter passed as a pointer and return 0/-1 properly. Probably nobody wanted to mess with half of libvirt code, I guess... Maybe... :) Luyao -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC v1 0/6] Live Migration with ephemeral host NIC devices
my main goal is to add support migration with host NIC passthrough devices and keep the network connectivity. this series patch base on Shradha's patches on https://www.redhat.com/archives/libvir-list/2012-November/msg01324.html which is add migration support for host passthrough devices. 1) unplug the ephemeral devices before migration 2) do native migration 3) when migration finished, hotplug the ephemeral devices TODO: keep network connectivity on guest level by bonding device. Chen Fan (6): conf: add ephemeral element for hostdev supporting migration qemu: Save ephemeral devices into qemuDomainObjPrivate qemu: add check ephemeral devices only for PCI host devices migration: Migration support for ephemeral hostdevs managedsave: move the domain xml handling forward to stop CPU managedsave: add managedsave support for ephemeral host devices docs/schemas/domaincommon.rng | 10 ++ docs/schemas/network.rng | 5 + src/conf/domain_conf.c | 14 +- src/conf/domain_conf.h | 1 + src/conf/network_conf.c| 13 ++ src/conf/network_conf.h| 1 + src/network/bridge_driver.c| 1 + src/qemu/qemu_command.c| 11 ++ src/qemu/qemu_domain.c | 5 + src/qemu/qemu_domain.h | 3 + src/qemu/qemu_driver.c | 48 +++--- src/qemu/qemu_migration.c | 182 - src/qemu/qemu_migration.h | 9 + src/qemu/qemu_process.c| 12 ++ tests/networkxml2xmlin/hostdev-pf.xml | 2 +- tests/networkxml2xmlin/hostdev.xml | 2 +- tests/networkxml2xmlout/hostdev-pf.xml | 2 +- tests/networkxml2xmlout/hostdev.xml| 2 +- .../qemuxml2argv-controller-order.xml | 2 +- .../qemuxml2argv-hostdev-pci-address-device.xml| 2 +- .../qemuxml2argv-hostdev-pci-address.xml | 2 +- .../qemuxml2argv-hostdev-scsi-autogen-address.xml | 22 +-- .../qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.xml | 4 +- .../qemuxml2argv-hostdev-scsi-lsi-iscsi.xml| 4 +- .../qemuxml2argv-hostdev-scsi-lsi.xml | 2 +- .../qemuxml2argv-hostdev-scsi-rawio.xml| 2 +- .../qemuxml2argv-hostdev-scsi-readonly.xml | 2 +- .../qemuxml2argv-hostdev-scsi-sgio.xml | 2 +- .../qemuxml2argv-hostdev-scsi-shareable.xml| 2 +- ...qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.xml | 4 +- .../qemuxml2argv-hostdev-scsi-virtio-iscsi.xml | 4 +- .../qemuxml2argv-hostdev-scsi-virtio-scsi.xml | 2 +- ...emuxml2argv-hostdev-usb-address-device-boot.xml | 2 +- .../qemuxml2argv-hostdev-usb-address-device.xml| 2 +- .../qemuxml2argv-hostdev-usb-address.xml | 2 +- .../qemuxml2argv-hostdev-vfio-multidomain.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-hostdev-vfio.xml | 2 +- .../qemuxml2argv-net-hostdev-multidomain.xml | 2 +- .../qemuxml2argv-net-hostdev-vfio-multidomain.xml | 2 +- .../qemuxml2argv-net-hostdev-vfio.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-net-hostdev.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-pci-rom.xml| 4 +- ...qemuxml2xmlout-hostdev-scsi-autogen-address.xml | 22 +-- 43 files changed, 340 insertions(+), 83 deletions(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC v1 1/6] conf: add ephemeral element for hostdev supporting migration
the ephemeral flag helps support migration with PCI-passthrough. An ephemeral hostdev is automatically unplugged before migration and replugged (if one is available on the destination) after migration. Signed-off-by: Chen Fan --- docs/schemas/domaincommon.rng | 10 ++ docs/schemas/network.rng | 5 + src/conf/domain_conf.c | 14 +- src/conf/domain_conf.h | 1 + src/conf/network_conf.c| 13 + src/conf/network_conf.h| 1 + src/network/bridge_driver.c| 1 + src/qemu/qemu_command.c| 1 + tests/networkxml2xmlin/hostdev-pf.xml | 2 +- tests/networkxml2xmlin/hostdev.xml | 2 +- tests/networkxml2xmlout/hostdev-pf.xml | 2 +- tests/networkxml2xmlout/hostdev.xml| 2 +- .../qemuxml2argv-controller-order.xml | 2 +- .../qemuxml2argv-hostdev-pci-address-device.xml| 2 +- .../qemuxml2argv-hostdev-pci-address.xml | 2 +- .../qemuxml2argv-hostdev-scsi-autogen-address.xml | 22 +++--- .../qemuxml2argv-hostdev-scsi-lsi-iscsi-auth.xml | 4 ++-- .../qemuxml2argv-hostdev-scsi-lsi-iscsi.xml| 4 ++-- .../qemuxml2argv-hostdev-scsi-lsi.xml | 2 +- .../qemuxml2argv-hostdev-scsi-rawio.xml| 2 +- .../qemuxml2argv-hostdev-scsi-readonly.xml | 2 +- .../qemuxml2argv-hostdev-scsi-sgio.xml | 2 +- .../qemuxml2argv-hostdev-scsi-shareable.xml| 2 +- ...qemuxml2argv-hostdev-scsi-virtio-iscsi-auth.xml | 4 ++-- .../qemuxml2argv-hostdev-scsi-virtio-iscsi.xml | 4 ++-- .../qemuxml2argv-hostdev-scsi-virtio-scsi.xml | 2 +- ...emuxml2argv-hostdev-usb-address-device-boot.xml | 2 +- .../qemuxml2argv-hostdev-usb-address-device.xml| 2 +- .../qemuxml2argv-hostdev-usb-address.xml | 2 +- .../qemuxml2argv-hostdev-vfio-multidomain.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-hostdev-vfio.xml | 2 +- .../qemuxml2argv-net-hostdev-multidomain.xml | 2 +- .../qemuxml2argv-net-hostdev-vfio-multidomain.xml | 2 +- .../qemuxml2argv-net-hostdev-vfio.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-net-hostdev.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-pci-rom.xml| 4 ++-- ...qemuxml2xmlout-hostdev-scsi-autogen-address.xml | 22 +++--- 37 files changed, 99 insertions(+), 55 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index b1d883f..6f4551c 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2261,6 +2261,11 @@ + + + + + @@ -3717,6 +3722,11 @@ + + + + + diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index 4edb6eb..d63b066 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -115,6 +115,11 @@ + + + + + diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3d05844..a1a0602 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4823,6 +4823,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, { xmlNodePtr sourcenode; char *managed = NULL; +char *ephemeral = NULL; char *sgio = NULL; char *rawio = NULL; char *backendStr = NULL; @@ -4841,6 +4842,11 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node, def->managed = true; } +if ((ephemeral = virXMLPropString(node, "ephemeral")) != NULL) { +if (STREQ(ephemeral, "yes")) +def->ephemeral = true; +} + sgio = virXMLPropString(node, "sgio"); rawio = virXMLPropString(node, "rawio"); @@ -18064,8 +18070,10 @@ virDomainActualNetDefFormat(virBufferPtr buf, virBufferAsprintf(buf, "managed) +if (hostdef && hostdef->managed) virBufferAddLit(buf, " managed='yes'"); +if (hostdef && hostdef->ephemeral) +virBufferAddLit(buf, " ephemeral='yes'"); } if (def->trustGuestRxFilters) virBufferAsprintf(buf, " trustGuestRxFilters='%s'", @@ -18236,6 +18244,8 @@ virDomainNetDefFormat(virBufferPtr buf, virBufferAsprintf(buf, "managed) virBufferAddLit(buf, " managed='yes'"); +if (hostdef && hostdef->ephemeral) +virBufferAddLit(buf, " ephemeral='yes'"); if (def->trustGuestRxFilters) virBufferAsprintf(buf, " trustGuestRxFilters='%s'", virTristateBoolTypeToString(def->trustGuestRxFilters)); @@ -19562,6 +19572,8 @@
[libvirt] [RFC v1 5/6] managedsave: move the domain xml handling forward to stop CPU
we should save the XML information to image head before we hotunplug the ephemeral devices. so here we handle XML ahead. Signed-off-by: Chen Fan --- src/qemu/qemu_driver.c | 40 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b3263ac..86d93d2 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3179,26 +3179,6 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom, priv->job.current->type = VIR_DOMAIN_JOB_UNBOUNDED; -/* Pause */ -if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { -was_running = true; -if (qemuProcessStopCPUs(driver, vm, VIR_DOMAIN_PAUSED_SAVE, -QEMU_ASYNC_JOB_SAVE) < 0) -goto endjob; - -if (!virDomainObjIsActive(vm)) { -virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("guest unexpectedly quit")); -goto endjob; -} -} - - /* libvirt.c already guaranteed these two flags are exclusive. */ -if (flags & VIR_DOMAIN_SAVE_RUNNING) -was_running = true; -else if (flags & VIR_DOMAIN_SAVE_PAUSED) -was_running = false; - /* Get XML for the domain. Restore needs only the inactive xml, * including secure. We should get the same result whether xmlin * is NULL or whether it was the live xml of the domain moments @@ -3225,6 +3205,26 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom, goto endjob; } +/* Pause */ +if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { +was_running = true; +if (qemuProcessStopCPUs(driver, vm, VIR_DOMAIN_PAUSED_SAVE, +QEMU_ASYNC_JOB_SAVE) < 0) +goto endjob; + +if (!virDomainObjIsActive(vm)) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("guest unexpectedly quit")); +goto endjob; +} +} + + /* libvirt.c already guaranteed these two flags are exclusive. */ +if (flags & VIR_DOMAIN_SAVE_RUNNING) +was_running = true; +else if (flags & VIR_DOMAIN_SAVE_PAUSED) +was_running = false; + ret = qemuDomainSaveMemory(driver, vm, path, xml, compressed, was_running, flags, QEMU_ASYNC_JOB_SAVE); if (ret < 0) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC v1 4/6] migration: Migration support for ephemeral hostdevs
add migration support for ephemeral host devices, introduce two 'detach' and 'restore' functions to unplug/plug host devices during migration. Signed-off-by: Chen Fan --- src/qemu/qemu_migration.c | 171 -- src/qemu/qemu_migration.h | 9 +++ src/qemu/qemu_process.c | 11 +++ 3 files changed, 187 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 56112f9..d5a698f 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3384,6 +3384,158 @@ qemuMigrationPrepareDef(virQEMUDriverPtr driver, return def; } +int +qemuMigrationDetachEphemeralDevices(virQEMUDriverPtr driver, +virDomainObjPtr vm, +bool live) +{ +qemuDomainObjPrivatePtr priv = vm->privateData; +virDomainHostdevDefPtr hostdev; +virDomainNetDefPtr net; +virDomainDeviceDef dev; +virDomainDeviceDefPtr dev_copy = NULL; +virCapsPtr caps = NULL; +int actualType; +int ret = -1; +size_t i; + +VIR_DEBUG("Rum domain detach ephemeral devices"); + +if (!(caps = virQEMUDriverGetCapabilities(driver, false))) +return ret; + +for (i = 0; i < vm->def->nnets;) { +net = vm->def->nets[i]; + +actualType = virDomainNetGetActualType(net); +if (actualType != VIR_DOMAIN_NET_TYPE_HOSTDEV) { +i++; +continue; +} + +hostdev = virDomainNetGetActualHostdev(net); +if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || +hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI || +!hostdev->ephemeral) { +i++; +continue; +} + +dev.type = VIR_DOMAIN_DEVICE_NET; +dev.data.net = net; + +dev_copy = virDomainDeviceDefCopy(&dev, vm->def, + caps, driver->xmlopt); +if (!dev_copy) +goto cleanup; + +if (live) { +/* nnets reduced */ +if (qemuDomainDetachNetDevice(driver, vm, dev_copy) < 0) +goto cleanup; +} else { +virDomainNetDefFree(virDomainNetRemove(vm->def, i)); +} +if (VIR_APPEND_ELEMENT(priv->ephemeralDevices, + priv->nEphemeralDevices, + dev_copy) < 0) { +goto cleanup; +} +dev_copy = NULL; +} + +for (i = 0; i < vm->def->nhostdevs;) { +hostdev = vm->def->hostdevs[i]; + +if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || +hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI || +!hostdev->ephemeral) { +i++; +continue; +} + +dev.type = VIR_DOMAIN_DEVICE_HOSTDEV; +dev.data.hostdev = hostdev; + +VIR_FREE(dev_copy); +dev_copy = virDomainDeviceDefCopy(&dev, vm->def, + caps, driver->xmlopt); +if (!dev_copy) +goto cleanup; + +if (live) { +/* nhostdevs reduced */ +if (qemuDomainDetachHostDevice(driver, vm, dev_copy) < 0) +goto cleanup; +} else { +virDomainHostdevDefFree(virDomainHostdevRemove(vm->def, i)); +} +if (VIR_APPEND_ELEMENT(priv->ephemeralDevices, + priv->nEphemeralDevices, + dev_copy) < 0) { +goto cleanup; +} +dev_copy = NULL; +} + +ret = 0; + cleanup: +virDomainDeviceDefFree(dev_copy); +virObjectUnref(caps); + +return ret; +} + +void +qemuMigrationRestoreEphemeralDevices(virQEMUDriverPtr driver, + virConnectPtr conn, + virDomainObjPtr vm, + bool live) +{ +qemuDomainObjPrivatePtr priv = vm->privateData; +virDomainDeviceDefPtr dev; +int ret = -1; +size_t i; + +VIR_DEBUG("Rum domain restore ephemeral devices"); + +for (i = 0; i < priv->nEphemeralDevices; i++) { +dev = priv->ephemeralDevices[i]; + +switch ((virDomainDeviceType) dev->type) { +case VIR_DOMAIN_DEVICE_NET: +if (live) { +ret = qemuDomainAttachNetDevice(conn, driver, vm, +dev->data.net); +} else { +ret = virDomainNetInsert(vm->def, dev->data.net); +} + +if (!ret) +dev->data.net = NULL; +break; +case VIR_DOMAIN_DEVICE_HOSTDEV: +if (live) { +ret = qemuDomainAttachHostDevice(conn, driver, vm, + dev->data.hostdev); + } else { +ret =virDomainHostdevInsert(vm->def, dev->data.hostdev); +} +
[libvirt] [RFC v1 3/6] qemu: add check ephemeral devices only for PCI host devices
currently, we only support PCI host devices with ephemeral flag. and USB already supports migration. so maybe in the near future we can support SCSI. Signed-off-by: Chen Fan --- src/qemu/qemu_command.c | 10 ++ src/qemu/qemu_migration.c | 11 +++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fc81214..5acd8b4 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10182,6 +10182,16 @@ qemuBuildCommandLine(virConnectPtr conn, /* PCI */ if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && +hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB && +(hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && + hostdev->ephemeral)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("non-USB and non-PCI device assignment with ephemeral " + "flag are not supported by this version of qemu")); +goto error; +} + +if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { int backend = hostdev->source.subsys.u.pci.backend; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 83be435..56112f9 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1981,21 +1981,24 @@ qemuMigrationIsAllowed(virQEMUDriverPtr driver, virDomainObjPtr vm, def = vm->def; } -/* Migration with USB host devices is allowed, all other devices are - * forbidden. +/* + * Migration with USB and ephemeral PCI host devices host devices are allowed, + * all other devices are forbidden. */ forbid = false; for (i = 0; i < def->nhostdevs; i++) { virDomainHostdevDefPtr hostdev = def->hostdevs[i]; if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS || -hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB) { +(hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB && +!hostdev->ephemeral)) { forbid = true; break; } } if (forbid) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("domain has assigned non-USB host devices")); + _("domain has assigned non-USB and " + "non-ephemeral host devices")); return false; } -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC v1 6/6] managedsave: add managedsave support for ephemeral host devices
Signed-off-by: Chen Fan --- src/qemu/qemu_driver.c | 8 src/qemu/qemu_process.c | 3 ++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 86d93d2..112acb1 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3208,6 +3208,10 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom, /* Pause */ if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_RUNNING) { was_running = true; +/* Detach ephemeral host devices first */ +if (qemuMigrationDetachEphemeralDevices(driver, vm, true) < 0) +goto endjob; + if (qemuProcessStopCPUs(driver, vm, VIR_DOMAIN_PAUSED_SAVE, QEMU_ASYNC_JOB_SAVE) < 0) goto endjob; @@ -3249,6 +3253,8 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver, virDomainPtr dom, VIR_DOMAIN_EVENT_SUSPENDED, VIR_DOMAIN_EVENT_SUSPENDED_API_ERROR); } +qemuMigrationRestoreEphemeralDevices(driver, dom->conn, vm, true); + virSetError(save_err); virFreeError(save_err); } @@ -6404,6 +6410,8 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, if (event) qemuDomainEventQueue(driver, event); +/* Restore ephemeral devices */ +qemuMigrationRestoreEphemeralDevices(driver, NULL, vm, true); /* If it was running before, resume it now unless caller requested pause. */ if (header->was_running && !start_paused) { diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 904c447..6519477 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4501,7 +4501,8 @@ int qemuProcessStart(virConnectPtr conn, * during migration. hence we should remove the reserved * PCI address for ephemeral device. */ -if (vmop == VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START) +if (vmop == VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START || +vmop == VIR_NETDEV_VPORT_PROFILE_OP_RESTORE) if (qemuMigrationDetachEphemeralDevices(driver, vm, false) < 0) goto cleanup; -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC v1 2/6] qemu: Save ephemeral devices into qemuDomainObjPrivate
after migration we should restore the ephemeral devices. so we save them to qemuDomainObjPrivate. Signed-off-by: Chen Fan --- src/qemu/qemu_domain.c | 5 + src/qemu/qemu_domain.h | 3 +++ 2 files changed, 8 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d8a2087..5ce933d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -425,6 +425,7 @@ static void qemuDomainObjPrivateFree(void *data) { qemuDomainObjPrivatePtr priv = data; +size_t i; virObjectUnref(priv->qemuCaps); @@ -441,6 +442,10 @@ qemuDomainObjPrivateFree(void *data) virCondDestroy(&priv->unplugFinished); virChrdevFree(priv->devs); +for (i = 0; i < priv->nEphemeralDevices; i++) +virDomainDeviceDefFree(priv->ephemeralDevices[i]); +VIR_FREE(priv->ephemeralDevices); + /* This should never be non-NULL if we get here, but just in case... */ if (priv->mon) { VIR_ERROR(_("Unexpected QEMU monitor still active during domain deletion")); diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index fe3e2b1..e75c828 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -180,6 +180,9 @@ struct _qemuDomainObjPrivate { size_t ncleanupCallbacks; size_t ncleanupCallbacks_max; +virDomainDeviceDefPtr *ephemeralDevices; +size_t nEphemeralDevices; + virCgroupPtr cgroup; virCond unplugFinished; /* signals that unpluggingDevice was unplugged */ -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] time: event poll may become un-triggerable after changing system clock.
On 2015/5/12 16:56, Daniel P. Berrange wrote: > On Tue, May 12, 2015 at 04:14:37PM +0800, zhang bo wrote: >> So, Is there any other better solution? thanks in advance. > > Simply don't change the system time by massive deltas. Libvirt is not going > to be the only app to be affected. As you mention it is going to hit the > pthread_cond_wait() call which will likely affect pretty much every single > non-trivial process running on the system. I'd expect other apps have much > the same problem with calculating poll sleeps too. > > If you need to massively change the system time this should be done at > single user mode, or do a reboot. Once a system is running it should be > kept synced with NTPD which will only ever change system time in very > small increments and so once cause thsi problem. > > Regards, > Daniel Thank you Daniel. Would you please explain more about what "single user mode" mean here? and does "reboot" refers to "rebooting the os" or "rebooting the process libvirtd"? I think it's "the process", right? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Xen-devel] [PATCH 1/4] libxl: populate build_info vfb in separate function
Konrad Rzeszutek Wilk wrote: > On Fri, May 08, 2015 at 04:31:02PM -0600, Jim Fehlig wrote: > >> For HVM domains, vfb info must be populated in the libxl_domain_build_info >> stuct. Currently this is done in the libxlMakeVfbList function, but IMO >> > > struct > Thanks. I've fixed the typo in my local branch but will wait for additional comments before posting a V2 (if necessary). Regards, Jim >> it would be cleaner to populate the build_info vfb in a separate >> libxlMakeBuildInfoVfb function. libxlMakeVfbList would then handle only >> vfb devices, simiar to the other libxlMakeList functions. >> >> A future patch will extend libxlMakeBuildInfoVfb to support SPICE. >> >> Signed-off-by: Jim Fehlig >> --- >> src/libxl/libxl_conf.c | 79 >> ++ >> 1 file changed, 48 insertions(+), 31 deletions(-) >> >> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c >> index fccada5..8552c77 100644 >> --- a/src/libxl/libxl_conf.c >> +++ b/src/libxl/libxl_conf.c >> @@ -1323,37 +1323,6 @@ libxlMakeVfbList(virPortAllocatorPtr graphicsports, >> d_config->vkbs = x_vkbs; >> d_config->num_vfbs = d_config->num_vkbs = nvfbs; >> >> -/* >> - * VNC or SDL info must also be set in libxl_domain_build_info >> - * for HVM domains. Use the first vfb device. >> - */ >> -if (def->os.type == VIR_DOMAIN_OSTYPE_HVM) { >> -libxl_domain_build_info *b_info = &d_config->b_info; >> -libxl_device_vfb vfb = d_config->vfbs[0]; >> - >> -if (libxl_defbool_val(vfb.vnc.enable)) { >> -libxl_defbool_set(&b_info->u.hvm.vnc.enable, true); >> -if (VIR_STRDUP(b_info->u.hvm.vnc.listen, vfb.vnc.listen) < 0) >> -goto error; >> -if (VIR_STRDUP(b_info->u.hvm.vnc.passwd, vfb.vnc.passwd) < 0) >> -goto error; >> -b_info->u.hvm.vnc.display = vfb.vnc.display; >> -libxl_defbool_set(&b_info->u.hvm.vnc.findunused, >> - libxl_defbool_val(vfb.vnc.findunused)); >> -} else if (libxl_defbool_val(vfb.sdl.enable)) { >> -libxl_defbool_set(&b_info->u.hvm.sdl.enable, true); >> -libxl_defbool_set(&b_info->u.hvm.sdl.opengl, >> - libxl_defbool_val(vfb.sdl.opengl)); >> -if (VIR_STRDUP(b_info->u.hvm.sdl.display, vfb.sdl.display) < 0) >> -goto error; >> -if (VIR_STRDUP(b_info->u.hvm.sdl.xauthority, >> vfb.sdl.xauthority) < 0) >> -goto error; >> -} >> - >> -if (VIR_STRDUP(b_info->u.hvm.keymap, vfb.keymap) < 0) >> -goto error; >> -} >> - >> return 0; >> >> error: >> @@ -1367,6 +1336,51 @@ libxlMakeVfbList(virPortAllocatorPtr graphicsports, >> } >> >> /* >> + * Populate vfb info in libxl_domain_build_info struct for HVM domains. >> + * Use first libxl_device_vfb device in libxl_domain_config->vfbs. >> + * Prior to calling this function, libxlMakeVfbList must be called to >> + * populate libxl_domain_config->vfbs. >> + */ >> +static int >> +libxlMakeBuildInfoVfb(virDomainDefPtr def, libxl_domain_config *d_config) >> +{ >> +libxl_domain_build_info *b_info = &d_config->b_info; >> +libxl_device_vfb x_vfb; >> + >> +if (def->os.type != VIR_DOMAIN_OSTYPE_HVM) >> +return 0; >> + >> +if (d_config->num_vfbs == 0) >> +return 0; >> + >> +x_vfb = d_config->vfbs[0]; >> + >> +if (libxl_defbool_val(x_vfb.vnc.enable)) { >> +libxl_defbool_set(&b_info->u.hvm.vnc.enable, true); >> +if (VIR_STRDUP(b_info->u.hvm.vnc.listen, x_vfb.vnc.listen) < 0) >> +return -1; >> +if (VIR_STRDUP(b_info->u.hvm.vnc.passwd, x_vfb.vnc.passwd) < 0) >> +return -1; >> +b_info->u.hvm.vnc.display = x_vfb.vnc.display; >> +libxl_defbool_set(&b_info->u.hvm.vnc.findunused, >> + libxl_defbool_val(x_vfb.vnc.findunused)); >> +} else if (libxl_defbool_val(x_vfb.sdl.enable)) { >> +libxl_defbool_set(&b_info->u.hvm.sdl.enable, true); >> +libxl_defbool_set(&b_info->u.hvm.sdl.opengl, >> + libxl_defbool_val(x_vfb.sdl.opengl)); >> +if (VIR_STRDUP(b_info->u.hvm.sdl.display, x_vfb.sdl.display) < 0) >> +return -1; >> +if (VIR_STRDUP(b_info->u.hvm.sdl.xauthority, x_vfb.sdl.xauthority) >> < 0) >> +return -1; >> +} >> + >> +if (VIR_STRDUP(b_info->u.hvm.keymap, x_vfb.keymap) < 0) >> +return -1; >> + >> +return 0; >> +} >> + >> +/* >> * Get domain0 autoballoon configuration. Honor user-specified >> * setting in libxl.conf first. If not specified, autoballooning >> * is disabled when domain0's memory is set with 'dom0_mem'. >> @@ -1764,6 +1778,9 @@ libxlBuildDomainConfig(virPortAllocatorPtr >> graphicsports, >> if (libxlMakeVfbList(graphicsports, def, d_config) < 0) >> return -1; >> >> +if (libx
Re: [libvirt] [PATCH] libxl: support VNC passwd
Martin Kletzander wrote: > On Fri, May 08, 2015 at 04:44:14PM -0600, Jim Fehlig wrote: >> While implementing support for SPICE, noticed VNC passwd was never > > s/, n/, I n/ ? > >> copied to libxl_device_vfb's vnc.passwd field. >> > > ACK Thanks! I fixed the nit and pushed. Regards, Jim > >> Signed-off-by: Jim Fehlig >> --- >> src/libxl/libxl_conf.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c >> index fccada5..35b1d04 100644 >> --- a/src/libxl/libxl_conf.c >> +++ b/src/libxl/libxl_conf.c >> @@ -1283,6 +1283,8 @@ libxlMakeVfb(virPortAllocatorPtr graphicsports, >> if (VIR_STRDUP(x_vfb->vnc.listen, listenAddr) < 0) >> return -1; >> } >> +if (VIR_STRDUP(x_vfb->vnc.passwd, >> l_vfb->data.vnc.auth.passwd) < 0) >> +return -1; >> if (VIR_STRDUP(x_vfb->keymap, l_vfb->data.vnc.keymap) < 0) >> return -1; >> break; >> -- >> 1.8.4.5 >> >> -- >> libvir-list mailing list >> libvir-list@redhat.com >> https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Taint domains using cdrom-passthrough
https://bugzilla.redhat.com/show_bug.cgi?id=976387 For a domain configured using the host cdrom, we should taint the domain due to problems encountered when the host and guest try to control the tray. Signed-off-by: John Ferlan --- src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + src/qemu/qemu_domain.c | 6 ++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index add857c..a67e200 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -101,7 +101,8 @@ VIR_ENUM_IMPL(virDomainTaint, VIR_DOMAIN_TAINT_LAST, "disk-probing", "external-launch", "host-cpu", - "hook-script"); + "hook-script", + "cdrom-passthrough"); VIR_ENUM_IMPL(virDomainVirt, VIR_DOMAIN_VIRT_LAST, "qemu", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2cd105a7..0867e8b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2280,6 +2280,7 @@ typedef enum { VIR_DOMAIN_TAINT_EXTERNAL_LAUNCH, /* Externally launched guest domain */ VIR_DOMAIN_TAINT_HOST_CPU, /* Host CPU passthrough in use */ VIR_DOMAIN_TAINT_HOOK, /* Domain (possibly) changed via hook script */ +VIR_DOMAIN_TAINT_CDROM_PASSTHROUGH,/* CDROM passthrough */ VIR_DOMAIN_TAINT_LAST } virDomainTaintFlags; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index fa8229f..b66ee89 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2031,6 +2031,12 @@ void qemuDomainObjCheckDiskTaint(virQEMUDriverPtr driver, qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_HIGH_PRIVILEGES, logFD); +if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM && +virStorageSourceGetActualType(disk->src) == VIR_STORAGE_TYPE_BLOCK && +disk->src->path) +qemuDomainObjTaint(driver, obj, VIR_DOMAIN_TAINT_CDROM_PASSTHROUGH, + logFD); + virObjectUnref(cfg); } -- 2.1.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 09/13] qemu: use controller object alias in commandline for virtio-serial device
On 05/12/2015 11:16 AM, Laine Stump wrote: > On 05/08/2015 08:05 PM, John Ferlan wrote: >> >> On 05/05/2015 02:03 PM, Laine Stump wrote: >>> The commandline generator for a virtio-serial device had a hardcoded >>> printf of "virtio-serial%d" as the id of the virtio-serial >>> controller. This patch changes it to use the alias of the controller >>> instead. Because the function that finds a controller alias requires a >>> pointer to the domainDef in order to get the list of controllers, the >>> arglist of a few functions had to have this added. >>> >>> Once this was done, the literal string QEMU_VIRTIO_SERIAL_PREFIX was >>> no longer needed, so it has been removed. >>> --- >>> src/qemu/qemu_command.c | 28 +--- >>> src/qemu/qemu_command.h | 1 - >>> 2 files changed, 17 insertions(+), 12 deletions(-) >>> >> ACK - although a couple of notes (a cscope search on "bus=" finds : >> >> qemuUSBId >> >> - where there are decision points to use usb or usb%d > > Yeah. It looks like that is another exception (so another place where > the status XML isn't correctly reflecting the id used in qemu). > > >> qemuBuildSCSIHostdevDevStr >> >> - where bus=scsi%d is used >> > > > Yes, this one should be changed too. > > Also, there is a place in qemuBuildDeviceAddressStr() where it is making > decisions about the name to use for the PCI bus based on whether or not > PCI_MULTIBUS is true. Instead, the function that builds the alias for > controllers should create the correct name for the one-and-only PCI > controller in this case, and qemuBuildDeviceAddressStr() should just > unconditionally use that name. > > Should I merge all of these into a single patch? Or a separate patch for > each? > > I'm fine with a single patch being submitted - although perhaps "for review purposes" - combine 7-9 as one... then post the new areas with the mindset to merge the patches before pushing. That way it makes it easier to review/explain the new changes. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] sysinfo: Fix reports on arm
On 05/12/2015 10:23 AM, Michal Privoznik wrote: > Due to a commit in kernel (155597223) it's 'processor' rather than > 'Processor'. Fix our parser too. Shouldn't we be parsing this string case-insensitively, to work with kernels that pre-date that kernel commit? > > Signed-off-by: Michal Privoznik > --- > src/util/virsysinfo.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c > index 8bb17f0..fb8cb2c 100644 > --- a/src/util/virsysinfo.c > +++ b/src/util/virsysinfo.c > @@ -289,7 +289,7 @@ virSysinfoParseProcessor(const char *base, > virSysinfoDefPtr ret) > virSysinfoProcessorDefPtr processor; > char *processor_type = NULL; > > -if (!(tmp_base = strstr(base, "Processor"))) > +if (!(tmp_base = strstr(base, "processor"))) > return 0; > > base = tmp_base; > -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] virSysinfoParseProcessor: Drop useless check for NULL
On 05/12/2015 10:23 AM, Michal Privoznik wrote: > VIR_STRDUP plays nicely with NULLs. Theres no need to guard its > call with check for non-NULL. > > Signed-off-by: Michal Privoznik > --- > src/util/virsysinfo.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) ACK > > diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c > index 4edce66..8bb17f0 100644 > --- a/src/util/virsysinfo.c > +++ b/src/util/virsysinfo.c > @@ -315,8 +315,7 @@ virSysinfoParseProcessor(const char *base, > virSysinfoDefPtr ret) > cur, eol - cur) < 0) > goto error; > > -if (processor_type && > -VIR_STRDUP(processor->processor_type, processor_type) < 0) > +if (VIR_STRDUP(processor->processor_type, processor_type) < 0) > goto error; > > base = cur; > -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: vnc: error out for invalid port number
In the XML we have the vnc port number, but QEMU takes on command line a vnc screen number, it's port-5900. We should fail with error message that only ports in range [5900,65535] are valid. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1164966 Signed-off-by: Pavel Hrdina --- src/qemu/qemu_command.c | 96 +++-- 1 file changed, 53 insertions(+), 43 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8ccbe82..5d0a167 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7591,50 +7591,60 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, virBufferAsprintf(&opt, "unix:%s", graphics->data.vnc.socket); -} else if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_COLON)) { -switch (virDomainGraphicsListenGetType(graphics, 0)) { -case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: -listenAddr = virDomainGraphicsListenGetAddress(graphics, 0); -break; - -case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: -listenNetwork = virDomainGraphicsListenGetNetwork(graphics, 0); -if (!listenNetwork) -break; -ret = networkGetNetworkAddress(listenNetwork, &netAddr); -if (ret <= -2) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - "%s", _("network-based listen not possible, " - "network driver not present")); -goto error; -} -if (ret < 0) -goto error; - -listenAddr = netAddr; -/* store the address we found in the element so it will - * show up in status. */ -if (virDomainGraphicsListenSetAddress(graphics, 0, - listenAddr, -1, false) < 0) - goto error; -break; -} - -if (!listenAddr) -listenAddr = cfg->vncListen; - -escapeAddr = strchr(listenAddr, ':') != NULL; -if (escapeAddr) -virBufferAsprintf(&opt, "[%s]", listenAddr); -else -virBufferAdd(&opt, listenAddr, -1); -virBufferAsprintf(&opt, ":%d", - graphics->data.vnc.port - 5900); - -VIR_FREE(netAddr); } else { -virBufferAsprintf(&opt, "%d", - graphics->data.vnc.port - 5900); +if (!graphics->data.vnc.autoport && +(graphics->data.vnc.port < 5900 || + graphics->data.vnc.port > 65535)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("vnc port must be in range [5900,65535]")); +goto error; +} + +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_COLON)) { +switch (virDomainGraphicsListenGetType(graphics, 0)) { +case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS: +listenAddr = virDomainGraphicsListenGetAddress(graphics, 0); +break; + +case VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK: +listenNetwork = virDomainGraphicsListenGetNetwork(graphics, 0); +if (!listenNetwork) +break; +ret = networkGetNetworkAddress(listenNetwork, &netAddr); +if (ret <= -2) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + "%s", _("network-based listen not possible, " + "network driver not present")); +goto error; +} +if (ret < 0) +goto error; + +listenAddr = netAddr; +/* store the address we found in the element so it + * will show up in status. */ +if (virDomainGraphicsListenSetAddress(graphics, 0, + listenAddr, -1, false) < 0) +goto error; +break; +} + +if (!listenAddr) +listenAddr = cfg->vncListen; + +escapeAddr = strchr(listenAddr, ':') != NULL; +if (escapeAddr) +virBufferAsprintf(&opt, "[%s]", listenAddr); +else +virBufferAdd(&opt, listenAddr, -1); +virBufferAsprintf(&opt, ":%d", + graphics->data.vnc.port - 5900); + +VIR_FREE(netAddr); +} else { +virBufferAsprintf(&opt, "%d", + graphics->data.vnc.port - 5900); +} } if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNC_COLON)) { -- 2.3.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] cpuset / numa and qemu in TCG mode
On Tue, May 12, 2015 at 11:14:09AM +0200, Martin Kletzander wrote: > On Tue, May 12, 2015 at 05:27:34PM +1000, Tony Breeds wrote: > >On Mon, May 11, 2015 at 01:14:58PM +0200, Martin Kletzander wrote: > > > >>Determining this by version might not be reliable, but more > >>importantly working around bug in underlying software is something > >>that shouldn't be done at all IMHO. Let the maintainers backport > >>whatever needs to be done. > > > >I agree with you in an ideal world but there are times when we do need > >to add work arounds in $project_x to work around issues in $project_y. > > > >>>Ther nova side will be pretty easy regardless. > >>> > >>>I'd say the best solution would be to back port the 'fix' but that seems > >>>like a > >>>lot of effort given the number of distros and libvirt versions potentiall > >>>involved. > >>> > >> > >>If you want the fix to be distro-agnostic, there's nothing easier than > >>back-porting the fix into our upstream maintenance branches. Those > >>should make the life of distro maintainers easy (although it looks > >>like not many distros use it). > > > >And this is part of the problem. If I understand correctly Ubuntu > >cloud-archive > >is using libvirt 1.2.12 which is *NOT* a maintenance release so that leaves > >us > >with doing an additional backport to 1.2.12 and getting the cloud-archive > >team > >to take it[1] or Adding a hack to nova. And that's just Ubuntu It's hard to > >say for sure that some vendor isn't running libvirt 1.2.12 also. > > > >>Having said that I'm not sure which commit(s) are those that need to > >>be back-ported. Having known your libvirt version, it shouldn't be > >>too hard looking for the differences and finding the right commit. > >>When back-porting request is made on the list, it is usually acted > >>upon. If you can't find the exact commit, let me know and I'll do my > >>best to help. > > > >So a git bisect points at: > >--- > >commit a103bb105c0c189c3973311ff1826972b5bc6ad6 > >Author: Daniel P. Berrange > >Date: Tue Feb 10 15:59:57 2015 + > > > > qemu: fix setting of VM CPU affinity with TCG > >--- > > > >A small amount of reading implies to me that we'd be looking at backporting > >a103bb105c0c189c3973311ff1826972b5bc6ad6 to any maintenance branch that > >contains > >b07f3d821dfb11a118ee75ea275fd6ab737d9500. Which I think is 1.2.13 only, but > >I > >could be wrong. > > > > 1.2.13 has the commit already in the release and 1.2.12-maint has it > as a first back-port right after release. The problem is that there > was no maintenance release of 1.2.12 yet. Maybe they would use > 1.2.12.1 if it existed. > > I Cc'd Guido as an upstream debian maintainer, maybe he'll have some > insights. @Guido: would it help if we created a maintenance release > from the v1.2.12-maint branch? Or is the only thing missing the fact > that the launchpad bug is not moved to libvirt? Basically Ubuntu takes the version that is in Debian testing, unstable or experimental at time of their release and adds pathes. There's little to no sync unfortunately (except for the awesome efforts to sync the apparmor stuff) Debian itself is not using 1.2.12 anywhere. Current stable has 1.2.9 + maintenance patches (which I intend to switch over to the maintenance releases soonish and support hopefully Cole with these), oldstable has 0.9.12.3 and unstable/sid has 1.2.15 (and will keep following the releases). I've added Serge since he might to jump onto 1.2.12.1 maintenance relasese. I'm happy about any additional notifications for things that should go into distributions. Cheers, -- Guido -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] virSysinfoParseProcessor: Drop useless check for NULL
VIR_STRDUP plays nicely with NULLs. Theres no need to guard its call with check for non-NULL. Signed-off-by: Michal Privoznik --- src/util/virsysinfo.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 4edce66..8bb17f0 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -315,8 +315,7 @@ virSysinfoParseProcessor(const char *base, virSysinfoDefPtr ret) cur, eol - cur) < 0) goto error; -if (processor_type && -VIR_STRDUP(processor->processor_type, processor_type) < 0) +if (VIR_STRDUP(processor->processor_type, processor_type) < 0) goto error; base = cur; -- 2.3.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Couple of sysinfo fixes
I've just tried sysinfo on my RPi, and found some bugs there. Michal Privoznik (2): virSysinfoParseProcessor: Drop useless check for NULL sysinfo: Fix reports on arm src/util/virsysinfo.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) -- 2.3.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] sysinfo: Fix reports on arm
Due to a commit in kernel (155597223) it's 'processor' rather than 'Processor'. Fix our parser too. Signed-off-by: Michal Privoznik --- src/util/virsysinfo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c index 8bb17f0..fb8cb2c 100644 --- a/src/util/virsysinfo.c +++ b/src/util/virsysinfo.c @@ -289,7 +289,7 @@ virSysinfoParseProcessor(const char *base, virSysinfoDefPtr ret) virSysinfoProcessorDefPtr processor; char *processor_type = NULL; -if (!(tmp_base = strstr(base, "Processor"))) +if (!(tmp_base = strstr(base, "processor"))) return 0; base = tmp_base; -- 2.3.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 8/9] qemu: Add quorum support in qemuBuildDriveDevStr
On Thu, Apr 23, 2015 at 14:41:20 +0200, Matthias Gatto wrote: > Allow to libvirt to build the quorum string used by quemu. > > Add 2 static functions: qemuBuildQuorumStr and > qemuBuildAndAppendDriveStrToVirBuffer. > qemuBuildQuorumStr is made because a quorum can have another quorum > as a child, so we may need to call qemuBuildQuorumStr recursively. > > qemuBuildQuorumFileSourceStr was basically made to share > the code use to build the source between qemuBuildQuorumStr and > qemuBuildDriveStr, but there is some difference betwin the syntax > use by libvirt to declare a disk and the one qemu need to build a quorum: > a quorum need a syntaxe like: > "domaine_name.children.X.file.filename=filename" > where libvirt don't use "file.filename=" but directly "file=". > Therfore I use this function only for quorum. > > But as explained in the cover letter and here: > http://www.redhat.com/archives/libvir-list/2014-October/msg00529.html > We miss some informations in _virStorageSource to have a complet > quorum support in libvirt. > Ideally I'd like to refactore virDomainDiskDefFormat to allow > qemuBuildQuorumStr to call this function in a loop. > > Signed-off-by: Matthias Gatto > --- > src/qemu/qemu_command.c | 110 > > 1 file changed, 110 insertions(+) > ... > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index 6c40d3e..80cbb7d 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -3479,6 +3479,111 @@ qemuCheckDiskConfig(virDomainDiskDefPtr disk) > return -1; > } > > +static bool > +qemuBuildQuorumFileSourceStr(virConnectPtr conn, > + virStorageSourcePtr src, > + virBuffer *opt, > + const char *toAppend) > +{ > +char *source = NULL; > +int actualType = virStorageSourceGetActualType(src); > + > +if (qemuGetDriveSourceString(src, conn, &source) < 0) > +goto error; > + > +if (source) { > + > +virBufferAsprintf(opt, ",%sfilename=", toAppend); virBufferStrcat > + > +switch (actualType) { > +case VIR_STORAGE_TYPE_DIR: I'd forbid the DIR type altogether with quorums. > +/* QEMU only supports magic FAT format for now */ > +if (src->format > 0 && > +src->format != VIR_STORAGE_FILE_FAT) { > +virReportError(VIR_ERR_INTERNAL_ERROR, > + _("unsupported disk driver type for '%s'"), > + > virStorageFileFormatTypeToString(src->format)); > +goto error; > +} > + > +if (!src->readonly) { > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("cannot create virtual FAT disks in > read-write mode")); > +goto error; > +} > + > +virBufferAddLit(opt, "fat:"); > + > +break; > + > +default: > +break; > +} > +virBufferAsprintf(opt, "%s", source); virBufferAdd > +} > + > +return true; > + error: > +return false; Error can be returned right away since there is nothing to clean up. > +} > + > + > +static bool > +qemuBuildQuorumStr(virConnectPtr conn, > + virDomainDiskDefPtr disk, > + virStorageSourcePtr src, > + virBuffer *opt, > + const char *toAppend) > +{ > +char *tmp = NULL; > +int ret; > +virStorageSourcePtr backingStore; > +size_t i; > + > +if (!src->threshold) { > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("threshold missing in the quorum configuration")); > +return false; > +} > +if (src->nBackingStores < 2) { > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("a quorum must have at last 2 children")); > +return false; > +} > +if (src->threshold > src->nBackingStores) { > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > + _("threshold must not exceed the number of > childrens")); 'children' is the proper plural > +return false; > +} > +virBufferAsprintf(opt, ",%svote-threshold=%lu", > + toAppend, src->threshold); > +for (i = 0; i < src->nBackingStores; ++i) { > +backingStore = virStorageSourceGetBackingStore(src, i); > +ret = virAsprintf(&tmp, "%schildren.%lu.file.", toAppend, i); > +if (ret < 0) > +return false; > + > +virBufferAsprintf(opt, ",%schildren.%lu.driver=%s", > + toAppend, i, > + > virStorageFileFormatTypeToString(backingStore->format)); > + > +if (qemuBuildQuorumFileSourceStr(conn, backingStore, opt, tmp) == > false) > +goto error; > + > +/* This operation avoid me t
Re: [libvirt] [PATCH 0/4] Enable support for s390 crypto key mgmt operations
On 04/27/2015 05:57 PM, akrow...@linux.vnet.ibm.com wrote: From: Tony Krowiak The IBM System z Central Processor Assist for Cryptographic Functions (CPACF) hardware provides a set of CPU instructions for use in clear-key encryption, pseudo random number generation, hash functions, and protected-key encryption. The CPACF protected key cryptographic functions operate with a protected key which is encrypted under a unique wrapping key that is stored in the Hardware System Area (HSA) of the machine and can only be accessed by firmware. The wrapping key cannot be accessed by the operating system or application programs. There are two wrapping keys: One for wrapping AES keys and one for wrapping DES/TDES keys. This patch set enables the support for encrypting clear keys under the AES and DES/TDES wrapping keys for guests started on hosts running on s390 hardware that supports key wrapping. Tony Krowiak (4): libvirt: docs: XML to enable/disable protected key mgmt ops libvirt: conf: parse XML for protected key management ops libvirt: qemu: enable/disable protected key management ops libvirt: tests: test protected key mgmt ops support -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list Any comments? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 09/13] qemu: use controller object alias in commandline for virtio-serial device
On 05/08/2015 08:05 PM, John Ferlan wrote: > > On 05/05/2015 02:03 PM, Laine Stump wrote: >> The commandline generator for a virtio-serial device had a hardcoded >> printf of "virtio-serial%d" as the id of the virtio-serial >> controller. This patch changes it to use the alias of the controller >> instead. Because the function that finds a controller alias requires a >> pointer to the domainDef in order to get the list of controllers, the >> arglist of a few functions had to have this added. >> >> Once this was done, the literal string QEMU_VIRTIO_SERIAL_PREFIX was >> no longer needed, so it has been removed. >> --- >> src/qemu/qemu_command.c | 28 +--- >> src/qemu/qemu_command.h | 1 - >> 2 files changed, 17 insertions(+), 12 deletions(-) >> > ACK - although a couple of notes (a cscope search on "bus=" finds : > > qemuUSBId > > - where there are decision points to use usb or usb%d Yeah. It looks like that is another exception (so another place where the status XML isn't correctly reflecting the id used in qemu). > qemuBuildSCSIHostdevDevStr > > - where bus=scsi%d is used > Yes, this one should be changed too. Also, there is a place in qemuBuildDeviceAddressStr() where it is making decisions about the name to use for the PCI bus based on whether or not PCI_MULTIBUS is true. Instead, the function that builds the alias for controllers should create the correct name for the one-and-only PCI controller in this case, and qemuBuildDeviceAddressStr() should just unconditionally use that name. Should I merge all of these into a single patch? Or a separate patch for each? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: fix double free when fail to cold-plug a rng device
On 12.05.2015 15:55, Luyao Huang wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=1220809 > > When cold-plug a rng device and get failed in qemuDomainAssignAddresses, > we will double free the rng device. Free the pointer after we Insert the > device success to fix this issue. > > ... > 5 0x7fb7d180ac8a in virFree at util/viralloc.c:582 > 6 0x7fb7d1895cdd in virDomainRNGDefFree at conf/domain_conf.c:19786 > 7 0x7fb7d1895d99 in virDomainDeviceDefFree at conf/domain_conf.c:2022 > 8 0x7fb7b92b8baf in qemuDomainAttachDeviceFlags at > qemu/qemu_driver.c:8785 > 9 0x7fb7d190c5d7 in virDomainAttachDeviceFlags at libvirt-domain.c:8488 > 10 0x7fb7d23af9d2 in remoteDispatchDomainAttachDeviceFlags at > remote_dispatch.h:2842 > ... > > Signed-off-by: Luyao Huang > --- > src/qemu/qemu_driver.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 33c1cfd..f922a28 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -8359,11 +8359,10 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, > > if (virDomainRNGInsert(vmdef, dev->data.rng, false) < 0) > return -1; > +dev->data.rng = NULL; > > if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) > return -1; > - > -dev->data.rng = NULL; > break; > > case VIR_DOMAIN_DEVICE_MEMORY: > I've reworded the commit message a bit, ACKed and pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [v3 0/2] Remove host name checking from iSCSI duplicate source checks
On 12.05.2015 04:23, John Ferlan wrote: > Only a v3 because the first two series "Addition host name check for > network storage pools" were not well received. If someone wants to > pick up/use the the hostname checking code from the v2, then have at it. > Patch 1, 2, 7, & 8 use the virSocketAddr code. > > This series just focuses on the iSCSI duplicate source checks and in > particular the removal of the host name checks to be replaced primarily > by the duplicate source device path (or IQN) check. For iSCSI devices, > checking the resolved host name was not feasible. Instead, if the source > device path (IQN) of the new definition is the same as a running pool, > then the new definition will be rejected. The secondary check is for an > undocumented element to define the initiator iqn (see bz 488142). > > The first patch is "new" - it's something I discovered while doing some > extra testing with the v2 of the series. > > The second patch provides the change and modifies the documentation. It > resolves two bz's as listed in the patch description. > > v2 here: > http://www.redhat.com/archives/libvir-list/2015-April/msg01197.html > > Much discussion took place in v1 though in 7/7: > http://www.redhat.com/archives/libvir-list/2015-April/msg00880.html > > John Ferlan (2): > conf: Adjust duplicate source host port check > conf: Remove source host name check for iSCSI > > docs/formatstorage.html.in | 18 ++ > docs/storage.html.in | 8 +++- > src/conf/storage_conf.c| 7 +++ > 3 files changed, 24 insertions(+), 9 deletions(-) > ACK series. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 7/9] domain_conf: Read and Write quorum config
On Thu, Apr 23, 2015 at 14:41:19 +0200, Matthias Gatto wrote: > Add the capabiltty to libvirt to parse and format the quorum syntax > as described here: > http://www.redhat.com/archives/libvir-list/2014-May/msg00533.html > > Signed-off-by: Matthias Gatto > --- > src/conf/domain_conf.c | 164 > +++-- > 1 file changed, 119 insertions(+), 45 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index a3a6c13..ec93b6f 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -5952,20 +5952,56 @@ virDomainDiskSourceParse(xmlNodePtr node, > } > > > +static bool > +virDomainDiskThresholdParse(virStorageSourcePtr src, > +xmlNodePtr node) > +{ > +char *threshold = virXMLPropString(node, "threshold"); > +int ret; > + > +if (!threshold) { > +virReportError(VIR_ERR_XML_ERROR, > + "%s", _("missing threshold in quorum")); > +return false; > +} > +ret = virStrToLong_ul(threshold, NULL, 10, &src->threshold); > +if (ret < 0 || src->threshold < 2) { > +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("unexpected threshold %s"), > + "threshold must be a decimal number superior to 2 " > + "and inferior to the number of children"); > +VIR_FREE(threshold); > +return false; > +} > +VIR_FREE(threshold); > +return true; > +} > + > + > static int > virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, > - virStorageSourcePtr src) > + virStorageSourcePtr src, > + xmlNodePtr node, > + size_t pos) > { > virStorageSourcePtr backingStore = NULL; > xmlNodePtr save_ctxt = ctxt->node; > -xmlNodePtr source; > +xmlNodePtr source = NULL; > char *type = NULL; > char *format = NULL; > int ret = -1; > > -if (!(ctxt->node = virXPathNode("./backingStore[*]", ctxt))) { > -ret = 0; > -goto cleanup; > +if (src->type == VIR_STORAGE_TYPE_QUORUM) { > +if (!node) { > +ret = 0; > +goto cleanup; > +} > +ctxt->node = node; > +} else { > +if (!(ctxt->node = virXPathNode("./backingStore[*]", ctxt))) { > +ret = 0; > +goto cleanup; > +} > } > > if (VIR_ALLOC(backingStore) < 0) > @@ -5997,6 +6033,25 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, > goto cleanup; > } > > +if (backingStore->type == VIR_STORAGE_TYPE_QUORUM) { > +xmlNodePtr cur = NULL; > + > +if (!virDomainDiskThresholdParse(backingStore, node)) > +goto cleanup; > + > +for (cur = node->children; cur != NULL; cur = cur->next) { > +if (xmlStrEqual(cur->name, BAD_CAST "backingStore")) { > +if ((virDomainDiskBackingStoreParse(ctxt, > +backingStore, > +cur, > + > backingStore->nBackingStores) < 0)) { > +goto cleanup; > +} > +} > +} > +goto exit; > +} > + > if (!(source = virXPathNode("./source", ctxt))) { > virReportError(VIR_ERR_XML_ERROR, "%s", > _("missing disk backing store source")); > @@ -6004,10 +6059,11 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr > ctxt, > } > > if (virDomainDiskSourceParse(source, ctxt, backingStore) < 0 || > -virDomainDiskBackingStoreParse(ctxt, backingStore) < 0) > +virDomainDiskBackingStoreParse(ctxt, backingStore, NULL, 0) < 0) > goto cleanup; > > -if (!virStorageSourceSetBackingStore(src, backingStore, 0)) > + exit: > +if (!virStorageSourceSetBackingStore(src, backingStore, pos)) > goto cleanup; > ret = 0; > > @@ -6020,7 +6076,6 @@ virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt, > return ret; > } > > - > #define VENDOR_LEN 8 > #define PRODUCT_LEN 16 > > @@ -6518,6 +6573,10 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, > } > } else if (xmlStrEqual(cur->name, BAD_CAST "boot")) { > /* boot is parsed as part of virDomainDeviceInfoParseXML */ > +} else if (xmlStrEqual(cur->name, BAD_CAST "backingStore")) { > +if (virDomainDiskBackingStoreParse(ctxt, def->src, cur, > + def->src->nBackingStores) > < 0) > +goto error; > } > } > cur = cur->next; > @@ -6541,12 +6600,19 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, > def->device = VIR_DOMAIN_DISK_DEVICE_DISK; > } >
[libvirt] [PATCH 3/3] virSysinfo: Introduce SMBIOS type 2 support
https://bugzilla.redhat.com/show_bug.cgi?id=1220527 This type of information defines attributes of a system baseboard. With one caveat: in qemu they call it family, while in the specification it's referred to as type. I'm sticking with the latter. Signed-off-by: Michal Privoznik --- docs/formatdomain.html.in | 37 +- docs/schemas/domaincommon.rng | 24 src/conf/domain_conf.c | 57 + src/libvirt_private.syms| 1 + src/qemu/qemu_command.c | 48 +++ src/util/virsysinfo.c | 160 +++- src/util/virsysinfo.h | 14 +++ tests/qemuxml2argvdata/qemuxml2argv-smbios.args | 2 + tests/qemuxml2argvdata/qemuxml2argv-smbios.xml | 9 ++ 9 files changed, 346 insertions(+), 6 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index e0b6ba7..dc92aa3 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -375,6 +375,13 @@Virt-Manager 0.9.4 ++ ... @@ -435,11 +442,31 @@ family Identify the family a particular computer belongs to. -NB: Incorrectly supplied entries in either the bios -or system blocks will be ignored without error. -Other than uuid validation and date -format checking, all values are passed as strings to the -hypervisor driver. + + baseBoard + +This is block 2 of SMBIOS, with entry names drawn from: + +manufacturer +Manufacturer of BIOS +product +Product Name +version +Version of the product +serial +Serial number +asset +Asset tag +location +Location in chassis +type +Board type + +NB: Incorrectly supplied entries in either the bios or +system or baseBoard blocks will be +ignored without error. Other than uuid validation and +date format checking, all values are passed as strings +to the hypervisor driver. diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index c151e92..2bc84b5 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4240,6 +4240,18 @@ + + + + + + + + + + + + @@ -4265,6 +4277,18 @@ + + + manufacturer + product + version + serial + asset + location + type + + + [a-zA-Z0-9/\-_\. \(\)]+ diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 209416d..0f41375 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10997,6 +10997,52 @@ virSysinfoSystemParseXML(xmlNodePtr node, return ret; } +static int +virSysinfoBaseBoardParseXML(xmlNodePtr node, +xmlXPathContextPtr ctxt, +virSysinfoBaseBoardDefPtr *baseBoard) +{ +int ret = -1; +virSysinfoBaseBoardDefPtr def; + +if (!xmlStrEqual(node->name, BAD_CAST "baseBoard")) { +virReportError(VIR_ERR_XML_ERROR, "%s", + _("XML does not contain expected 'baseBoard' element")); +return ret; +} + +if (VIR_ALLOC(def) < 0) +goto cleanup; + +def->manufacturer = +virXPathString("string(entry[@name='manufacturer'])", ctxt); +def->product = +virXPathString("string(entry[@name='product'])", ctxt); +def->version = +virXPathString("string(entry[@name='version'])", ctxt); +def->serial = +virXPathString("string(entry[@name='serial'])", ctxt); +def->asset = +virXPathString("string(entry[@name='asset'])", ctxt); +def->location = +virXPathString("string(entry[@name='location'])", ctxt); +def->type = +virXPathString("string(entry[@name='type'])", ctxt); + +if (!def->manufacturer && !def->product && !def->version && +!def->serial && !def->asset && !def->location && !def->type) { +virSysinfoBaseBoardDefFree(def); +def = NULL; +} + +*baseBoard = def; +def = NULL; +LENOVO +20BE0061MC +0B98401 Pro +W1KS427111E +Motherboard +
[libvirt] [PATCH 1/3] virSysinfoDef: Exempt BIOS variables
Move all the bios_* fields into a separate struct. Not only this simplifies the code a bit it also helps us to identify whether BIOS info is present. We don't have to check all the four variables for being not-NULL, but we can just check the pointer to the struct. Signed-off-by: Michal Privoznik --- src/conf/domain_conf.c | 109 +--- src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 23 +- src/util/virsysinfo.c| 114 +++ src/util/virsysinfo.h| 15 +-- 5 files changed, 181 insertions(+), 81 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index add857c..e37e453 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10863,46 +10863,28 @@ virDomainShmemDefParseXML(xmlNodePtr node, return ret; } -static virSysinfoDefPtr -virSysinfoParseXML(xmlNodePtr node, - xmlXPathContextPtr ctxt, - unsigned char *domUUID, - bool uuid_generated) +static int +virSysinfoBIOSParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, + virSysinfoBIOSDefPtr *bios) { -virSysinfoDefPtr def; -char *type; -char *tmpUUID = NULL; +int ret = -1; +virSysinfoBIOSDefPtr def; -if (!xmlStrEqual(node->name, BAD_CAST "sysinfo")) { +if (!xmlStrEqual(node->name, BAD_CAST "bios")) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("XML does not contain expected 'sysinfo' element")); -return NULL; + _("XML does not contain expected 'bios' element")); +return ret; } if (VIR_ALLOC(def) < 0) -return NULL; +goto cleanup; -type = virXMLPropString(node, "type"); -if (type == NULL) { -virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("sysinfo must contain a type attribute")); -goto error; -} -if ((def->type = virSysinfoTypeFromString(type)) < 0) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown sysinfo type '%s'"), type); -goto error; -} - - -/* Extract BIOS related metadata */ -def->bios_vendor = -virXPathString("string(bios/entry[@name='vendor'])", ctxt); -def->bios_version = -virXPathString("string(bios/entry[@name='version'])", ctxt); -def->bios_date = -virXPathString("string(bios/entry[@name='date'])", ctxt); -if (def->bios_date != NULL) { +def->vendor = virXPathString("string(entry[@name='vendor'])", ctxt); +def->version = virXPathString("string(entry[@name='version'])", ctxt); +def->date = virXPathString("string(entry[@name='date'])", ctxt); +def->release = virXPathString("string(entry[@name='release'])", ctxt); +if (def->date != NULL) { char *ptr; int month, day, year; @@ -10911,7 +10893,7 @@ virSysinfoParseXML(xmlNodePtr node, * where yy must be 00->99 and would be assumed to be 19xx * a date should be 1900 and beyond */ -if (virStrToLong_i(def->bios_date, &ptr, 10, &month) < 0 || +if (virStrToLong_i(def->date, &ptr, 10, &month) < 0 || *ptr != '/' || virStrToLong_i(ptr + 1, &ptr, 10, &day) < 0 || *ptr != '/' || @@ -10922,11 +10904,66 @@ virSysinfoParseXML(xmlNodePtr node, (year < 0 || (year >= 100 && year < 1900))) { virReportError(VIR_ERR_XML_DETAIL, "%s", _("Invalid BIOS 'date' format")); +goto cleanup; +} +} + +if (!def->vendor && !def->version && +!def->date && !def->release) { +virSysinfoBIOSDefFree(def); +def = NULL; +} + +*bios = def; +def = NULL; +ret = 0; + cleanup: +virSysinfoBIOSDefFree(def); +return ret; +} + +static virSysinfoDefPtr +virSysinfoParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, + unsigned char *domUUID, + bool uuid_generated) +{ +virSysinfoDefPtr def; +xmlNodePtr oldnode, tmpnode; +char *type; +char *tmpUUID = NULL; + +if (!xmlStrEqual(node->name, BAD_CAST "sysinfo")) { +virReportError(VIR_ERR_XML_ERROR, "%s", + _("XML does not contain expected 'sysinfo' element")); +return NULL; +} + +if (VIR_ALLOC(def) < 0) +return NULL; + +type = virXMLPropString(node, "type"); +if (type == NULL) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("sysinfo must contain a type attribute")); +goto error; +} +if ((def->type = virSysinfoTypeFromString(type)) < 0) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown sysinfo type '%s'"), type); +goto error; +} + +/* Extract BIOS related metadata */ +if ((tmpnode = virXPathNode(
[libvirt] [PATCH 2/3] virSysinfoDef: Exempt SYSTEM variables
Move all the system_* fields into a separate struct. Not only this simplifies the code a bit it also helps us to identify whether BIOS info is present. We don't have to check all the four variables for being not-NULL, but we can just check the pointer to the struct. Signed-off-by: Michal Privoznik --- src/conf/domain_conf.c | 152 ++-- src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 41 src/util/virsysinfo.c| 258 --- src/util/virsysinfo.h| 22 ++-- 5 files changed, 312 insertions(+), 162 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e37e453..209416d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -10922,66 +10922,42 @@ virSysinfoBIOSParseXML(xmlNodePtr node, return ret; } -static virSysinfoDefPtr -virSysinfoParseXML(xmlNodePtr node, - xmlXPathContextPtr ctxt, - unsigned char *domUUID, - bool uuid_generated) +static int +virSysinfoSystemParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, + virSysinfoSystemDefPtr *system, + unsigned char *domUUID, + bool uuid_generated) { -virSysinfoDefPtr def; -xmlNodePtr oldnode, tmpnode; -char *type; +int ret = -1; +virSysinfoSystemDefPtr def; char *tmpUUID = NULL; -if (!xmlStrEqual(node->name, BAD_CAST "sysinfo")) { +if (!xmlStrEqual(node->name, BAD_CAST "system")) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("XML does not contain expected 'sysinfo' element")); -return NULL; + _("XML does not contain expected 'system' element")); +return ret; } if (VIR_ALLOC(def) < 0) -return NULL; +goto cleanup; -type = virXMLPropString(node, "type"); -if (type == NULL) { -virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("sysinfo must contain a type attribute")); -goto error; -} -if ((def->type = virSysinfoTypeFromString(type)) < 0) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown sysinfo type '%s'"), type); -goto error; -} - -/* Extract BIOS related metadata */ -if ((tmpnode = virXPathNode("./bios[1]", ctxt)) != NULL) { -oldnode = ctxt->node; -ctxt->node = tmpnode; -if (virSysinfoBIOSParseXML(tmpnode, ctxt, &def->bios) < 0) { -ctxt->node = oldnode; -goto error; -} -ctxt->node = oldnode; -} - -/* Extract system related metadata */ -def->system_manufacturer = -virXPathString("string(system/entry[@name='manufacturer'])", ctxt); -def->system_product = -virXPathString("string(system/entry[@name='product'])", ctxt); -def->system_version = -virXPathString("string(system/entry[@name='version'])", ctxt); -def->system_serial = -virXPathString("string(system/entry[@name='serial'])", ctxt); -tmpUUID = virXPathString("string(system/entry[@name='uuid'])", ctxt); +def->manufacturer = +virXPathString("string(entry[@name='manufacturer'])", ctxt); +def->product = +virXPathString("string(entry[@name='product'])", ctxt); +def->version = +virXPathString("string(entry[@name='version'])", ctxt); +def->serial = +virXPathString("string(entry[@name='serial'])", ctxt); +tmpUUID = virXPathString("string(entry[@name='uuid'])", ctxt); if (tmpUUID) { unsigned char uuidbuf[VIR_UUID_BUFLEN]; char uuidstr[VIR_UUID_STRING_BUFLEN]; if (virUUIDParse(tmpUUID, uuidbuf) < 0) { virReportError(VIR_ERR_XML_DETAIL, "%s", _("malformed uuid element")); -goto error; +goto cleanup; } if (uuid_generated) { memcpy(domUUID, uuidbuf, VIR_UUID_BUFLEN); @@ -10989,7 +10965,7 @@ virSysinfoParseXML(xmlNodePtr node, virReportError(VIR_ERR_XML_DETAIL, "%s", _("UUID mismatch between and " "")); -goto error; +goto cleanup; } /* Although we've validated the UUID as good, virUUIDParse() is * lax with respect to allowing extraneous "-" and " ", but the @@ -10998,17 +10974,85 @@ virSysinfoParseXML(xmlNodePtr node, * properly so that it's used correctly later. */ virUUIDFormat(uuidbuf, uuidstr); -if (VIR_STRDUP(def->system_uuid, uuidstr) < 0) -goto error; +if (VIR_STRDUP(def->uuid, uuidstr) < 0) +goto cleanup; } -def->system_sku = -virXPathString("string(system/entry[@name='sku'])", ctxt); -def->system_family = -virXPathString("string(system/entry[@name='family'])", ctxt); +
[libvirt] [PATCH 0/3] Introduce yet another type to SMBIOS
The specification is here: http://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.0.0.pdf The first two patches rework the code a bit, and the last one adds something useful. Michal Privoznik (3): virSysinfoDef: Exempt BIOS variables virSysinfoDef: Exempt SYSTEM variables virSysinfo: Introduce SMBIOS type 2 support docs/formatdomain.html.in | 37 +- docs/schemas/domaincommon.rng | 24 ++ src/conf/domain_conf.c | 252 --- src/libvirt_private.syms| 3 + src/qemu/qemu_command.c | 112 +++-- src/util/virsysinfo.c | 528 +++- src/util/virsysinfo.h | 51 ++- tests/qemuxml2argvdata/qemuxml2argv-smbios.args | 2 + tests/qemuxml2argvdata/qemuxml2argv-smbios.xml | 9 + 9 files changed, 804 insertions(+), 214 deletions(-) -- 2.3.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 5/9] virstoragefile: change backingStore to backingStores.
On Thu, Apr 23, 2015 at 14:41:17 +0200, Matthias Gatto wrote: > The backingStore field was a virStorageSourcePtr. > because a quorum can contain more that one backingStore at the same level > it's now a 'virStorageSourcePtr *'. > > This patch rename src->backingStore to src->backingStores, > add a static function virStorageSourceExpandBackingStore > (virStorageSourcePushBackingStore in the V2) and made the necessary > modification to > virStorageSourceSetBackingStore and virStorageSourceGetBackingStore. > virStorageSourceSetBackingStore can now expand size of src->backingStores > by calling virStorageSourceExpandBackingStore if necessary. > > Signed-off-by: Matthias Gatto > --- > src/storage/storage_backend.c| 2 +- > src/storage/storage_backend_fs.c | 2 +- > src/util/virstoragefile.c| 75 > +++- > src/util/virstoragefile.h| 3 +- > 4 files changed, 71 insertions(+), 11 deletions(-) > ... > diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c > index 234a72b..f0450b5 100644 > --- a/src/util/virstoragefile.c > +++ b/src/util/virstoragefile.c > @@ -1809,21 +1809,72 @@ virStorageSourcePoolDefCopy(const > virStorageSourcePoolDef *src) > } > > > +/** > + * virStorageSourceGetBackingStore: > + * @src: virStorageSourcePtr containing the backing stores > + * @pos: position of the backing store to get > + * > + * return the backingStore at the position of @pos > + */ > virStorageSourcePtr > -virStorageSourceGetBackingStore(const virStorageSource *src, > -size_t pos ATTRIBUTE_UNUSED) > +virStorageSourceGetBackingStore(const virStorageSource *src, size_t pos) > { > -return src->backingStore; > +if (!src || !src->backingStores || pos >= src->nBackingStores) > +return NULL; > +return src->backingStores[pos]; > } > > > +/** > + * virStorageSourcePushBackingStore: > + * > + * Expand src->backingStores and update src->nBackingStores > + */ > +static bool > +virStorageSourceExpandBackingStore(virStorageSourcePtr src, size_t nbr) > +{ > +if (!src) { > +virReportError(VIR_ERR_INTERNAL_ERROR, > + "%s", _("src is NULL")); > +return false; > +} > +if (src->nBackingStores > 0) { > +if (VIR_EXPAND_N(src->backingStores, src->nBackingStores, nbr) < 0) This internally reallocates the memory even if the original pointer is NULL, so there's no need for the if statement) > +goto allocation_failed; > +} else { > +if (VIR_ALLOC_N(src->backingStores, nbr) < 0) > +goto allocation_failed; > +src->nBackingStores += nbr; > +} > +return true; > + allocation_failed: > +virReportOOMError(); Almost every libvirt memory allocation function reports the OOM error internally, so there's no need to do it here. > +return false; > +} > + > + > +/** > + * virStorageSourceSetBackingStore: > + * @src: virStorageSourcePtr to hold @backingStore > + * @backingStore: backingStore to store > + * @pos: position of the backing store to store > + * > + * Set @backingStore at @pos in src->backingStores. > + * If src->backingStores don't have the space to contain > + * @backingStore, we expand src->backingStores > + */ > bool > virStorageSourceSetBackingStore(virStorageSourcePtr src, > virStorageSourcePtr backingStore, > -size_t pos ATTRIBUTE_UNUSED) > +size_t pos) > { > -src->backingStore = backingStore; > -return !!src->backingStore; > +if (!src) > +return false; > +if (pos >= src->nBackingStores && > +!virStorageSourceExpandBackingStore(src, pos - src->nBackingStores + > 1)) > +return false; > +src->backingStores[pos] = backingStore; > +return true; In general virStorageSourceExpandBackingStore is almost useless. It could be folded in this function. > } > > > @@ -2038,6 +2089,8 @@ virStorageSourceIsEmpty(virStorageSourcePtr src) > void > virStorageSourceBackingStoreClear(virStorageSourcePtr def) > { > +size_t i; > + > if (!def) > return; > > @@ -2045,8 +2098,14 @@ virStorageSourceBackingStoreClear(virStorageSourcePtr > def) > VIR_FREE(def->backingStoreRaw); > > /* recursively free backing chain */ > -virStorageSourceFree(virStorageSourceGetBackingStore(def, 0)); > -virStorageSourceSetBackingStore(def, NULL, 0); > +for (i = 0; i < def->nBackingStores; ++i) > +virStorageSourceFree(virStorageSourceGetBackingStore(def, i)); > +if (def->nBackingStores > 0) { > +/* in this case def->backingStores is treat as an array so we have > to free it*/ s/treat/treated/ ... or drop the comment > +VIR_FREE(def->backingStores); > +} > +def->nBackingStores = 0; > +def->backingStores = NULL; VIR_FREE sets the pointer to NULL already. > } > > In general, the code
[libvirt] [PATCH] qemu: fix double free when fail to cold-plug a rng device
https://bugzilla.redhat.com/show_bug.cgi?id=1220809 When cold-plug a rng device and get failed in qemuDomainAssignAddresses, we will double free the rng device. Free the pointer after we Insert the device success to fix this issue. ... 5 0x7fb7d180ac8a in virFree at util/viralloc.c:582 6 0x7fb7d1895cdd in virDomainRNGDefFree at conf/domain_conf.c:19786 7 0x7fb7d1895d99 in virDomainDeviceDefFree at conf/domain_conf.c:2022 8 0x7fb7b92b8baf in qemuDomainAttachDeviceFlags at qemu/qemu_driver.c:8785 9 0x7fb7d190c5d7 in virDomainAttachDeviceFlags at libvirt-domain.c:8488 10 0x7fb7d23af9d2 in remoteDispatchDomainAttachDeviceFlags at remote_dispatch.h:2842 ... Signed-off-by: Luyao Huang --- src/qemu/qemu_driver.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 33c1cfd..f922a28 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8359,11 +8359,10 @@ qemuDomainAttachDeviceConfig(virQEMUCapsPtr qemuCaps, if (virDomainRNGInsert(vmdef, dev->data.rng, false) < 0) return -1; +dev->data.rng = NULL; if (qemuDomainAssignAddresses(vmdef, qemuCaps, NULL) < 0) return -1; - -dev->data.rng = NULL; break; case VIR_DOMAIN_DEVICE_MEMORY: -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 9/9] virstoragefile: Add node-name
On Thu, Apr 23, 2015 at 14:41:21 +0200, Matthias Gatto wrote: > Add nodename inside virstoragefile > During xml backingStore parsing, look for a nodename attribute in the disk > declaration if this one is a quorum, if a nodename is found, add it to > the virStorageSource otherwise create a new one with a random name. > Take inspiration from this patch to create the nodename: > http://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03209.html > > Durring xml backingStore formating, look for a nodename attribute inside the > virStorageSource struct, and add it to the disk element. > > Use the nodename to create the quorum in qemuBuildQuorumStr. > > Signed-off-by: Matthias Gatto > --- Once we decide that we want to deal with node names (which we definitely should do soon we will need to take a different approach compared to this patch: > docs/formatdomain.html.in | 7 +++ > docs/schemas/domaincommon.rng | 5 + > src/conf/domain_conf.c| 27 +++ > src/qemu/qemu_command.c | 3 +++ > src/util/virstoragefile.c | 4 > src/util/virstoragefile.h | 1 + > 6 files changed, 47 insertions(+) > > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in > index 7d058ec..d9afe36 100644 > --- a/docs/formatdomain.html.in > +++ b/docs/formatdomain.html.in > @@ -2183,6 +2183,13 @@ > vda[2] refers to the backing store with > index='2' of the disk with vda target. > > + nodename attribute > + since 1.2.13 > + > +When the backing store is a quorum child, we can use this > attribute > +to define the node-name of a child. If this atribute is undefine, > +a random nodename is generate. We certainly don't want to give the user the need to specify node names. In fact I think libvirt shouldn't expose node names in any way. The implementation should remain internal and users will interact via the backing chain 'index' element: 'This attribute is only valid in output (and ignored on input) and it can be used to refer to a specific part of the disk chain when doing block operations (such as via the virDomainBlockRebase API). For example, vda[2] refers to the backing store with index='2' of the disk with vda target.' Once we do this we should specify a node name for every backing chain element or possibly re-detect it after qemu starts and store the backing chain info internally. This will be necessary as libvirt has to model the operations with the backing chain the same way as qemu is doing it so that libvirt can ensure that qemu is not accessing files that it should not access. At any rate, node names are a very useful concept, but this patch would be a step in the wrong direction. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: Keep track of what disks are being migrated
On Tue, May 12, 2015 at 13:53:20 +0100, Daniel P. Berrange wrote: > On Tue, May 12, 2015 at 02:37:09PM +0200, Jiri Denemark wrote: > > Instead of redoing the same filtering over and over everytime we need to > > walk through all disks which are being migrated. > > > > Signed-off-by: Jiri Denemark > > --- > > src/conf/domain_conf.h| 2 ++ > > src/qemu/qemu_migration.c | 23 ++- > > 2 files changed, 8 insertions(+), 17 deletions(-) > > > > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > > index 2cd105a7..391f49a 100644 > > --- a/src/conf/domain_conf.h > > +++ b/src/conf/domain_conf.h > > @@ -703,6 +703,8 @@ struct _virDomainDiskDef { > > int blockJobStatus; /* status of the finished block job */ > > bool blockJobSync; /* the block job needs synchronized termination */ > > > > +bool migrating; /* the disk is being migrated */ > > I'm not a huge fan of the fact that we're filling the generic > virDomainDiskDef struct with ever more QEMU driver state fields. Hmm, I think we could just introduce a per-disk priv where we would hide all this stuff. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix build --without-network
On Mon, Apr 27, 2015 at 03:49:35PM +0200, Martin Kletzander wrote: In order not to bring in any link dependencies, bridge driver doesn't use the usual stubs as other conditionally-built code does. However, having the function as a macro imposes a problem with possibly unused variables if just defined as "0". This was worked around by using (dom=dom, iface=iface, 0) which should act like a 0 if used in a condition. However, gcc still bugs about that, so I came up with another way how to fix that. Using static inline functions in the header won't collide with anything, it fixes the bug and does one thing that the macro didn't do. It checks whenther passed variables are pointers of compatible type. It has only one downside, and that is that we need to either a) define it with ATTRIBUTE_UNUSED, which needs an exception in cfg.mk or b) do something like ignore_value(variable); in the function body. I went with the first variant. Signed-off-by: Martin Kletzander --- Notes: I can go with the version (b) if that's the preferred one. Ping, should I send the second variant instead? Or change the whole build to compile it unconditionally with stubs added? cfg.mk | 4 ++-- src/network/bridge_driver.h | 19 --- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/cfg.mk b/cfg.mk index 9ba2134..796ed80 100644 --- a/cfg.mk +++ b/cfg.mk @@ -1,5 +1,5 @@ # Customize Makefile.maint. -*- makefile -*- -# Copyright (C) 2008-2014 Red Hat, Inc. +# Copyright (C) 2008-2015 Red Hat, Inc. # Copyright (C) 2003-2008 Free Software Foundation, Inc. # This program is free software: you can redistribute it and/or modify @@ -1184,7 +1184,7 @@ exclude_file_name_regexp--sc_prohibit_getenv = \ ^tests/.*\.[ch]$$ exclude_file_name_regexp--sc_avoid_attribute_unused_in_header = \ - ^src/util/virlog\.h$$ + ^(src/util/virlog\.h|src/network/bridge_driver\.h)$$ exclude_file_name_regexp--sc_prohibit_mixed_case_abbreviations = \ ^src/(vbox/vbox_CAPI.*.h|esx/esx_vi.(c|h)|esx/esx_storage_backend_iscsi.c)$$ diff --git a/src/network/bridge_driver.h b/src/network/bridge_driver.h index 2f801ee..513ccf7 100644 --- a/src/network/bridge_driver.h +++ b/src/network/bridge_driver.h @@ -1,7 +1,7 @@ /* * bridge_driver.h: core driver methods for managing networks * - * Copyright (C) 2006-2013 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -55,11 +55,24 @@ int networkDnsmasqConfContents(virNetworkObjPtr network, # else /* Define no-op replacements that don't drag in any link dependencies. */ # define networkAllocateActualDevice(dom, iface) 0 -# define networkNotifyActualDevice(dom, iface) (dom=dom, iface=iface, 0) -# define networkReleaseActualDevice(dom, iface) (dom=dom, iface=iface, 0) # define networkGetNetworkAddress(netname, netaddr) (-2) # define networkDnsmasqConfContents(network, pidfile, configstr, \ dctx, caps) 0 + +static inline int +networkNotifyActualDevice(virDomainDefPtr dom ATTRIBUTE_UNUSED, + virDomainNetDefPtr iface ATTRIBUTE_UNUSED) +{ +return 0; +} + +static inline int +networkReleaseActualDevice(virDomainDefPtr dom ATTRIBUTE_UNUSED, + virDomainNetDefPtr iface ATTRIBUTE_UNUSED) +{ +return 0; +} + # endif typedef char *(*networkDnsmasqLeaseFileNameFunc)(const char *netname); -- 2.3.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] qemu: Keep track of what disks are being migrated
On Tue, May 12, 2015 at 02:37:09PM +0200, Jiri Denemark wrote: > Instead of redoing the same filtering over and over everytime we need to > walk through all disks which are being migrated. > > Signed-off-by: Jiri Denemark > --- > src/conf/domain_conf.h| 2 ++ > src/qemu/qemu_migration.c | 23 ++- > 2 files changed, 8 insertions(+), 17 deletions(-) > > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index 2cd105a7..391f49a 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -703,6 +703,8 @@ struct _virDomainDiskDef { > int blockJobStatus; /* status of the finished block job */ > bool blockJobSync; /* the block job needs synchronized termination */ > > +bool migrating; /* the disk is being migrated */ I'm not a huge fan of the fact that we're filling the generic virDomainDiskDef struct with ever more QEMU driver state fields. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/5] qemu: migration: selective block device migration
On Tue, May 12, 2015 at 15:07:31 +0300, Pavel Boldin wrote: > Implement a `migrate_disks' parameters for the QEMU driver. This multi- > value parameter can be used to explicitly specify what block devices > are to be migrated using the NBD server. Tunnelled migration using NBD > is to be done. > ... > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c > index eb70f29..60c09d8 100644 > --- a/src/qemu/qemu_migration.c > +++ b/src/qemu/qemu_migration.c ... > @@ -1864,9 +1884,10 @@ qemuMigrationCancelDriveMirror(virQEMUDriverPtr driver, > for (i = 0; i < vm->def->ndisks; i++) { > virDomainDiskDefPtr disk = vm->def->disks[i]; > > -/* skip shared, RO and source-less disks */ > -if (disk->src->shared || disk->src->readonly || > -!virDomainDiskGetSource(disk)) > +/* check whether disk should be migrated */ > +/* TODO: pass migrate_disks here or loookup list of > + * disks under migration using some kind of qemu monitor command */ > +if (!qemuMigrateDisk(disk, NULL)) Some of the code in this patch including this TODO will not be necessary if https://www.redhat.com/archives/libvir-list/2015-May/msg00352.html is accepted... Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] qemu: Don't give up on first error in qemuMigrationCancelDriverMirror
When cancelling drive mirror, always try to do that for all disks even if it fails for some of them. Report the first error we saw. Signed-off-by: Jiri Denemark --- src/qemu/qemu_migration.c | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 7448794..2dce44b 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1861,6 +1861,8 @@ static int qemuMigrationCancelDriveMirror(virQEMUDriverPtr driver, virDomainObjPtr vm) { +virErrorPtr err = NULL; +int ret = 0; size_t i; for (i = 0; i < vm->def->ndisks; i++) { @@ -1869,13 +1871,20 @@ qemuMigrationCancelDriveMirror(virQEMUDriverPtr driver, if (!disk->migrating || !disk->blockJobSync) continue; -if (qemuMigrationCancelOneDriveMirror(driver, vm, disk) < 0) -return -1; +if (qemuMigrationCancelOneDriveMirror(driver, vm, disk) < 0) { +ret = -1; +if (!err) +err = virSaveLastError(); +} disk->migrating = false; } -return 0; +if (err) { +virSetError(err); +virFreeError(err); +} +return ret; } -- 2.4.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Cleanup storage migration code a bit
Jiri Denemark (2): qemu: Keep track of what disks are being migrated qemu: Don't give up on first error in qemuMigrationCancelDriverMirror src/conf/domain_conf.h| 2 ++ src/qemu/qemu_migration.c | 36 +--- 2 files changed, 19 insertions(+), 19 deletions(-) -- 2.4.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] qemu: Keep track of what disks are being migrated
Instead of redoing the same filtering over and over everytime we need to walk through all disks which are being migrated. Signed-off-by: Jiri Denemark --- src/conf/domain_conf.h| 2 ++ src/qemu/qemu_migration.c | 23 ++- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2cd105a7..391f49a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -703,6 +703,8 @@ struct _virDomainDiskDef { int blockJobStatus; /* status of the finished block job */ bool blockJobSync; /* the block job needs synchronized termination */ +bool migrating; /* the disk is being migrated */ + struct { unsigned int cylinders; unsigned int heads; diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index c1af704..7448794 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -1744,13 +1744,7 @@ qemuMigrationCheckDriveMirror(virQEMUDriverPtr driver, for (i = 0; i < vm->def->ndisks; i++) { virDomainDiskDefPtr disk = vm->def->disks[i]; -/* skip shared, RO and source-less disks */ -if (disk->src->shared || disk->src->readonly || -!virDomainDiskGetSource(disk)) -continue; - -/* skip disks that didn't start mirroring */ -if (!disk->blockJobSync) +if (!disk->migrating || !disk->blockJobSync) continue; /* process any pending event */ @@ -1872,17 +1866,13 @@ qemuMigrationCancelDriveMirror(virQEMUDriverPtr driver, for (i = 0; i < vm->def->ndisks; i++) { virDomainDiskDefPtr disk = vm->def->disks[i]; -/* skip shared, RO and source-less disks */ -if (disk->src->shared || disk->src->readonly || -!virDomainDiskGetSource(disk)) -continue; - -/* skip disks that didn't start mirroring */ -if (!disk->blockJobSync) +if (!disk->migrating || !disk->blockJobSync) continue; if (qemuMigrationCancelOneDriveMirror(driver, vm, disk) < 0) return -1; + +disk->migrating = false; } return 0; @@ -1973,6 +1963,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, qemuBlockJobSyncEnd(driver, vm, disk, NULL); goto cleanup; } +disk->migrating = true; } /* Wait for each disk to become ready in turn, but check the status @@ -1980,9 +1971,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver, for (i = 0; i < vm->def->ndisks; i++) { virDomainDiskDefPtr disk = vm->def->disks[i]; -/* skip shared, RO and source-less disks */ -if (disk->src->shared || disk->src->readonly || -!virDomainDiskGetSource(disk)) +if (!disk->migrating) continue; while (disk->mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_READY) { -- 2.4.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] daemon: logging: Suppress logging of VIR_ERR_NO_DOMAIN_METADATA
On Tue, May 12, 2015 at 14:05:36 +0200, Ján Tomko wrote: > The commit summary says logging twice. I've dropped the first instance ... > > On Tue, May 12, 2015 at 01:58:11PM +0200, Peter Krempa wrote: > > Similarly to other error codes that notify the user that the object does > > not exist lower the priority of VIR_ERR_NO_DOMAIN_METADATA to > > VIR_LOG_DEBUG when writing the log entry. > > --- > > daemon/libvirtd.c | 1 + > > 1 file changed, 1 insertion(+) > > ACK and pushed this patch. Thanks. Peter signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] util: multi-value virTypedParameter
The `virTypedParamsValidate' function now can be instructed to allow multiple entries for some of the keys. For this flag the type with the `VIR_TYPED_PARAM_MULTIPLE' flag. Add unit tests for this new behaviour. Signed-off-by: Pavel Boldin --- include/libvirt/libvirt-host.h | 8 ++ src/util/virtypedparam.c | 107 - tests/Makefile.am | 6 ++ tests/virtypedparamtest.c | 177 + 4 files changed, 259 insertions(+), 39 deletions(-) create mode 100644 tests/virtypedparamtest.c diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h index 953366b..a3e8b88 100644 --- a/include/libvirt/libvirt-host.h +++ b/include/libvirt/libvirt-host.h @@ -180,6 +180,14 @@ typedef enum { } virTypedParameterType; /** + * VIR_TYPED_PARAM_MULTIPLE: + * + * Flag indiciating that the params has multiple occurences of the parameter. + * Only used as a flag for @type argument of the virTypedParamsValidate. + */ +# define VIR_TYPED_PARAM_MULTIPLE (1 << 31) + +/** * virTypedParameterFlags: * * Flags related to libvirt APIs that use virTypedParameter. diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index de2d447..bd47609 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -47,11 +47,18 @@ VIR_ENUM_IMPL(virTypedParameter, VIR_TYPED_PARAM_LAST, * internal utility functions (those in libvirt_private.syms) may * report errors that the caller will dispatch. */ +static int _virTypedParamsSortName(const void *left, const void *right) +{ +const virTypedParameter *param_left = left, *param_right = right; +return strcmp(param_left->field, param_right->field); +} + /* Validate that PARAMS contains only recognized parameter names with - * correct types, and with no duplicates. Pass in as many name/type - * pairs as appropriate, and pass NULL to end the list of accepted - * parameters. Return 0 on success, -1 on failure with error message - * already issued. */ + * correct types, and with no duplicates except for parameters + * specified with VIR_TYPED_PARAM_MULTIPLE flag in type. + * Pass in as many name/type pairs as appropriate, and pass NULL to end + * the list of accepted parameters. Return 0 on success, -1 on failure + * with error message already issued. */ int virTypedParamsValidate(virTypedParameterPtr params, int nparams, ...) { @@ -60,60 +67,82 @@ virTypedParamsValidate(virTypedParameterPtr params, int nparams, ...) size_t i, j; const char *name; int type; +size_t nkeys = 0, nkeysmax = 0; +virTypedParameterPtr sorted = NULL, keys = NULL; va_start(ap, nparams); -/* Yes, this is quadratic, but since we reject duplicates and - * unknowns, it is constrained by the number of var-args passed - * in, which is expected to be small enough to not be - * noticeable. */ -for (i = 0; i < nparams; i++) { -va_end(ap); -va_start(ap, nparams); +if (VIR_ALLOC_N(sorted, nparams) < 0) +goto cleanup; -name = va_arg(ap, const char *); -while (name) { -type = va_arg(ap, int); -if (STREQ(params[i].field, name)) { -if (params[i].type != type) { -const char *badtype; - -badtype = virTypedParameterTypeToString(params[i].type); -if (!badtype) -badtype = virTypedParameterTypeToString(0); -virReportError(VIR_ERR_INVALID_ARG, - _("invalid type '%s' for parameter '%s', " - "expected '%s'"), - badtype, params[i].field, - virTypedParameterTypeToString(type)); -} -break; -} -name = va_arg(ap, const char *); -} -if (!name) { -virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, - _("parameter '%s' not supported"), - params[i].field); +/* Here we intentially don't copy values */ +memcpy(sorted, params, sizeof(*params) * nparams); +qsort(sorted, nparams, sizeof(*sorted), _virTypedParamsSortName); + +name = va_arg(ap, const char *); +while (name) { +type = va_arg(ap, int); +if (VIR_RESIZE_N(keys, nkeysmax, nkeys, 1) < 0) +goto cleanup; + +if (virStrcpyStatic(keys[nkeys].field, name) == NULL) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("Field name '%s' too long"), name); goto cleanup; } -for (j = 0; j < i; j++) { -if (STREQ(params[i].field, params[j].field)) { + +keys[nkeys].type = type & ~VIR_TYPED_PARAM_MULTIPLE; +/* Value is not used anyway */ +keys[nkeys].value.i = type & VIR_TYPED_PARAM_MULTIPLE; + +nkeys++; +name =
[libvirt] [PATCH 3/5] util: add virTypedParamsPackStrings
The `virTypedParamsPackStrings' function provides interface to pack multiple string values under the same key to the `virTypedParameter' array. Signed-off-by: Pavel Boldin --- include/libvirt/libvirt-host.h | 6 +++ src/libvirt_public.syms| 1 + src/util/virtypedparam.c | 94 +- tests/virtypedparamtest.c | 28 + 4 files changed, 109 insertions(+), 20 deletions(-) diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h index afa730f..090ac83 100644 --- a/include/libvirt/libvirt-host.h +++ b/include/libvirt/libvirt-host.h @@ -344,6 +344,12 @@ virTypedParamsAddString (virTypedParameterPtr *params, const char *name, const char *value); int +virTypedParamsPackStrings(virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *name, + const char **values); +int virTypedParamsAddFromString(virTypedParameterPtr *params, int *nparams, int *maxparams, diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index d886967..8a24bb6 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -714,6 +714,7 @@ LIBVIRT_1.2.16 { global: virTypedParamsPick; virTypedParamsPickStrings; +virTypedParamsPackStrings; } LIBVIRT_1.2.15; # define new API here using predicted next version number diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index 643d10e..9f2ab3c 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -1132,6 +1132,43 @@ virTypedParamsAddBoolean(virTypedParameterPtr *params, return -1; } +static int +virTypedParamsAddStringFull(virTypedParameterPtr *params, +int *nparams, +int *maxparams, +const char *name, +const char *value, +bool unique) +{ +char *str = NULL; +size_t max = *maxparams; +size_t n = *nparams; + +virResetLastError(); + +if (unique) +VIR_TYPED_PARAM_CHECK(); +if (VIR_RESIZE_N(*params, max, n, 1) < 0) +goto error; +*maxparams = max; + +if (VIR_STRDUP(str, value) < 0) +goto error; + +if (virTypedParameterAssign(*params + n, name, +VIR_TYPED_PARAM_STRING, str) < 0) { +VIR_FREE(str); +goto error; +} + +*nparams += 1; +return 0; + + error: +virDispatchError(NULL); +return -1; +} + /** * virTypedParamsAddString: @@ -1160,32 +1197,49 @@ virTypedParamsAddString(virTypedParameterPtr *params, const char *name, const char *value) { -char *str = NULL; -size_t max = *maxparams; -size_t n = *nparams; +return virTypedParamsAddStringFull(params, + nparams, + maxparams, + name, + value, + 1); +} -virResetLastError(); -VIR_TYPED_PARAM_CHECK(); -if (VIR_RESIZE_N(*params, max, n, 1) < 0) -goto error; -*maxparams = max; +/** + * virTypedParamsPackStrings: + * @params: array of typed parameters + * @nparams: number of parameters in the @params array + * @maxparams: maximum number of parameters that can be stored in @params + * array without allocating more memory + * @name: name of the parameter to store values to + * @values: the values to store into the new parameters + * + * Packs NULL-terminated list of strings @values into @params under the + * key @name. + * + * Returns 0 on success, -1 on error. + */ +int +virTypedParamsPackStrings(virTypedParameterPtr *params, + int *nparams, + int *maxparams, + const char *name, + const char **values) +{ +size_t i; +int rv = -1; -if (VIR_STRDUP(str, value) < 0) -goto error; +if (!values) +return 0; -if (virTypedParameterAssign(*params + n, name, -VIR_TYPED_PARAM_STRING, str) < 0) { -VIR_FREE(str); -goto error; +for (i = 0; values[i]; i++) { +if ((rv = virTypedParamsAddStringFull(params, nparams, maxparams, + name, values[i], 0)) < 0) +break; } -*nparams += 1; -return 0; - - error: -virDispatchError(NULL); -return -1; +return rv; } diff --git a/tests/virtypedparamtest.c b/tests/virtypedparamtest.c index 287d3f1..037d5d1 100644 --- a/tests/virtypedparamtest.c +++ b/tests/virtypedparamtest.c @@ -139,6 +139,31 @@ testTypedParamsPick(con
[libvirt] [PATCH 0/5] Selective block device migration implementation
The patchset represented in the mail thread implements the selective block device migration for the QEMU driver. This closes the referenced bug [1]. First the supplementary API implemented allowing for multiple key-values pair in the virTypedParameter arrays. This is used to pass the list of block devices to migrate in the following patches. Unit tests for this are added as well. This is the subject of the first three patches. Fourth patch is adding the necessary parameter `migrate_disks' and passes it through the QEMU driver call stack. The introduced `qemuMigrateBlockDevice' function is then checks that the disk is to be migrated because it is in the list. If there is no list specified then the legacy check is used: the device is not shared, not readonly and has a source. Fifth and the last patch is adding the necessary code to the `virsh' utility making it able for user to specify a comma-separated list of the block device names that are to be migrated. The implemented Python bindings patch is to be presented. [1] https://bugzilla.redhat.com/show_bug.cgi?id=1203032 Pavel Boldin (5): util: multi-value virTypedParameter util: virTypedParamsPick* multikey API util: add virTypedParamsPackStrings qemu: migration: selective block device migration virsh: selective block device migration include/libvirt/libvirt-domain.h | 9 ++ include/libvirt/libvirt-host.h | 24 src/libvirt_public.syms | 3 + src/qemu/qemu_driver.c | 63 ++--- src/qemu/qemu_migration.c| 197 -- src/qemu/qemu_migration.h| 23 ++-- src/util/virtypedparam.c | 290 +++ tests/Makefile.am| 6 + tests/virtypedparamtest.c| 289 ++ tools/virsh-domain.c | 44 ++ 10 files changed, 788 insertions(+), 160 deletions(-) create mode 100644 tests/virtypedparamtest.c -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/5] qemu: migration: selective block device migration
Implement a `migrate_disks' parameters for the QEMU driver. This multi- value parameter can be used to explicitly specify what block devices are to be migrated using the NBD server. Tunnelled migration using NBD is to be done. Signed-off-by: Pavel Boldin --- include/libvirt/libvirt-domain.h | 9 ++ src/qemu/qemu_driver.c | 63 + src/qemu/qemu_migration.c| 198 --- src/qemu/qemu_migration.h| 23 +++-- 4 files changed, 192 insertions(+), 101 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 0f465b9..6b48923 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -748,6 +748,15 @@ typedef enum { */ # define VIR_MIGRATE_PARAM_LISTEN_ADDRESS"listen_address" +/** + * VIR_MIGRATE_PARAM_MIGRATE_DISKS: + * + * virDomainMigrate* params multiple field: The multiple values that list + * the block devices to be migrated. At the moment this is only supported + * by the QEMU driver but not for the tunnelled migration. + */ +# define VIR_MIGRATE_PARAM_MIGRATE_DISKS"migrate_disks" + /* Domain migration. */ virDomainPtr virDomainMigrate (virDomainPtr domain, virConnectPtr dconn, unsigned long flags, const char *dname, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 3695b26..2f53df6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12472,7 +12472,7 @@ qemuDomainMigratePrepare2(virConnectPtr dconn, ret = qemuMigrationPrepareDirect(driver, dconn, NULL, 0, NULL, NULL, /* No cookies */ uri_in, uri_out, - &def, origname, NULL, flags); + &def, origname, NULL, flags, NULL); cleanup: VIR_FREE(origname); @@ -12525,7 +12525,7 @@ qemuDomainMigratePerform(virDomainPtr dom, * Consume any cookie we were able to decode though */ ret = qemuMigrationPerform(driver, dom->conn, vm, - NULL, dconnuri, uri, NULL, NULL, + NULL, dconnuri, uri, NULL, NULL, NULL, cookie, cookielen, NULL, NULL, /* No output cookies in v2 */ flags, dname, resource, false); @@ -12602,7 +12602,7 @@ qemuDomainMigrateBegin3(virDomainPtr domain, } return qemuMigrationBegin(domain->conn, vm, xmlin, dname, - cookieout, cookieoutlen, flags); + cookieout, cookieoutlen, flags, NULL); } static char * @@ -12615,11 +12615,13 @@ qemuDomainMigrateBegin3Params(virDomainPtr domain, { const char *xmlin = NULL; const char *dname = NULL; +const char **migrate_disks = NULL; +char *ret = NULL; virDomainObjPtr vm; virCheckFlags(QEMU_MIGRATION_FLAGS, NULL); if (virTypedParamsValidate(params, nparams, QEMU_MIGRATION_PARAMETERS) < 0) -return NULL; +goto cleanup; if (virTypedParamsGetString(params, nparams, VIR_MIGRATE_PARAM_DEST_XML, @@ -12627,18 +12629,25 @@ qemuDomainMigrateBegin3Params(virDomainPtr domain, virTypedParamsGetString(params, nparams, VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0) -return NULL; +goto cleanup; + +migrate_disks = virTypedParamsPickStrings(params, nparams, + VIR_MIGRATE_PARAM_MIGRATE_DISKS); if (!(vm = qemuDomObjFromDomain(domain))) -return NULL; +goto cleanup; if (virDomainMigrateBegin3ParamsEnsureACL(domain->conn, vm->def) < 0) { virDomainObjEndAPI(&vm); -return NULL; +goto cleanup; } -return qemuMigrationBegin(domain->conn, vm, xmlin, dname, - cookieout, cookieoutlen, flags); +ret = qemuMigrationBegin(domain->conn, vm, xmlin, dname, + cookieout, cookieoutlen, flags, migrate_disks); + + cleanup: +VIR_FREE(migrate_disks); +return ret; } @@ -12682,7 +12691,8 @@ qemuDomainMigratePrepare3(virConnectPtr dconn, cookiein, cookieinlen, cookieout, cookieoutlen, uri_in, uri_out, - &def, origname, NULL, flags); + &def, origname, NULL, flags, + NULL); cleanup: VIR_FREE(origname); @@ -12708,6 +12718,7 @@ qemuDomainMigratePrepare3Params(virConnectPtr dconn, const char *dname = NULL; const char *uri_in = NULL; const char *listenAddress = cfg->migrationAddress; +const char **migrate_disks = NULL; char *origname = NULL;
Re: [libvirt] [PATCH] daemon: logging: Suppress logging of VIR_ERR_NO_DOMAIN_METADATA
The commit summary says logging twice. On Tue, May 12, 2015 at 01:58:11PM +0200, Peter Krempa wrote: > Similarly to other error codes that notify the user that the object does > not exist lower the priority of VIR_ERR_NO_DOMAIN_METADATA to > VIR_LOG_DEBUG when writing the log entry. > --- > daemon/libvirtd.c | 1 + > 1 file changed, 1 insertion(+) ACK Jan > > diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c > index 107b88d..3e7f87c 100644 > --- a/daemon/libvirtd.c > +++ b/daemon/libvirtd.c > @@ -320,6 +320,7 @@ static int daemonErrorLogFilter(virErrorPtr err, int > priority) > case VIR_ERR_NO_SECRET: > case VIR_ERR_NO_DOMAIN_SNAPSHOT: > case VIR_ERR_OPERATION_INVALID: > +case VIR_ERR_NO_DOMAIN_METADATA: > return VIR_LOG_DEBUG; > } > signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/5] util: virTypedParamsPick* multikey API
Add multikey APIs for virTypedParams*: * virTypedParamsPick that returns all the parameters with the specified name and type. * virTypedParamsPickStrings that returns a NULL-terminated `const char**' list with all the values for specified name and string type. Signed-off-by: Pavel Boldin --- include/libvirt/libvirt-host.h | 10 + src/libvirt_public.syms| 6 +++ src/util/virtypedparam.c | 90 ++ tests/virtypedparamtest.c | 87 4 files changed, 193 insertions(+) diff --git a/include/libvirt/libvirt-host.h b/include/libvirt/libvirt-host.h index a3e8b88..afa730f 100644 --- a/include/libvirt/libvirt-host.h +++ b/include/libvirt/libvirt-host.h @@ -256,6 +256,12 @@ virTypedParameterPtr virTypedParamsGet (virTypedParameterPtr params, int nparams, const char *name); +virTypedParameterPtr* +virTypedParamsPick (virTypedParameterPtr params, + int nparams, + const char *name, + int type, + size_t *picked); int virTypedParamsGetInt(virTypedParameterPtr params, int nparams, @@ -291,6 +297,10 @@ virTypedParamsGetString (virTypedParameterPtr params, int nparams, const char *name, const char **value); +const char ** +virTypedParamsPickStrings(virTypedParameterPtr params, + int nparams, + const char *name); int virTypedParamsAddInt(virTypedParameterPtr *params, int *nparams, diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index ef3d2f0..d886967 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -710,4 +710,10 @@ LIBVIRT_1.2.15 { virDomainDelIOThread; } LIBVIRT_1.2.14; +LIBVIRT_1.2.16 { +global: +virTypedParamsPick; +virTypedParamsPickStrings; +} LIBVIRT_1.2.15; + # define new API here using predicted next version number diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index bd47609..643d10e 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -480,6 +480,56 @@ virTypedParamsGet(virTypedParameterPtr params, } +/** + * virTypedParamsPick: + * @params: array of typed parameters + * @nparams: number of parameters in the @params array + * @name: name of the parameter to find + * @type: type of the parameter to find + * @picked: pointer to a size_t with amount of picked + * + * + * Finds typed parameters called @name. + * + * Returns pointers to the parameters or NULL if there are none in @params. + * This function does not raise an error, even when returning NULL. + * Callee should call VIR_FREE on the returned array. + */ +virTypedParameterPtr* +virTypedParamsPick(virTypedParameterPtr params, + int nparams, + const char *name, + int type, + size_t *picked) +{ +size_t i, max = 0; +virTypedParameterPtr *values = NULL; + +*picked = 0; + +if (!params || !name) +return NULL; + +for (i = 0; i < nparams; i++) { +if (params[i].type == type && STREQ(params[i].field, name)) { +if (VIR_RESIZE_N(values, max, *picked, 1) < 0) +goto error; + +values[*picked] = ¶ms[i]; + +*picked += 1; +} +} + +return values; + + error: +*picked = 0; +VIR_FREE(values); +return NULL; +} + + #define VIR_TYPED_PARAM_CHECK_TYPE(check_type) \ do { if (param->type != check_type) { \ virReportError(VIR_ERR_INVALID_ARG, \ @@ -747,6 +797,46 @@ virTypedParamsGetString(virTypedParameterPtr params, } +/** + * virTypedParamsPickStrings: + * @params: array of typed parameters + * @nparams: number of parameters in the @params array + * @name: name of the parameter to find + * + * + * Finds typed parameters called @name. + * + * Returns pointers to the parameters or NULL if there are none in @params. + * This function does not raise an error, even when returning NULL. + * Callee should call VIR_FREE on the returned array. + */ +const char ** +virTypedParamsPickStrings(virTypedParameterPtr params, + int nparams, const char *name) +{ +const char **values = NULL; +size_t i, picked; +virTypedParameterPtr *picked_params; + +picked_params = virTypedParamsPick(params, nparams, + name, VIR_TYPED_PARAM_STRING, + &picked); + +if (picked_params == NULL) +return NULL; + +if (VIR_ALLOC_N(values, picked + 1) < 0) +goto cleanup; + +for (i = 0; i < picked; i++) +values[i
[libvirt] [PATCH 5/5] virsh: selective block device migration
Add `virsh migrate' option `--migratedisks' that allows CLI user to explicitly specify block devices to migrate. Signed-off-by: Pavel Boldin --- tools/virsh-domain.c | 43 +++ 1 file changed, 43 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 4b627e1..4f43a25 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -9793,6 +9793,10 @@ static const vshCmdOptDef opts_migrate[] = { .type = VSH_OT_STRING, .help = N_("filename containing updated XML for the target") }, +{.name = "migratedisks", + .type = VSH_OT_STRING, + .help = N_("comma separated list of disks to be migrated") +}, {.name = NULL} }; @@ -9852,6 +9856,45 @@ doMigrate(void *opaque) VIR_MIGRATE_PARAM_DEST_NAME, opt) < 0) goto save_error; +if (vshCommandOptStringReq(ctl, cmd, "migratedisks", &opt) < 0) +goto out; +if (opt) { +const char **val = NULL; +char *tok, *saveptr = NULL, *opt_copy = NULL, *optp; +size_t max = 0, n = 0; + +if (VIR_STRDUP(opt_copy, opt) < 0) +goto save_error; + +optp = opt_copy; +do { +tok = strtok_r(optp, ",", &saveptr); +optp = NULL; + +if (VIR_RESIZE_N(val, max, n, 1) < 0) { +VIR_FREE(opt_copy); +VIR_FREE(val); +goto save_error; +} + +val[n] = tok; +n++; +} while (tok != NULL); + +if (virTypedParamsPackStrings(¶ms, + &nparams, + &maxparams, + VIR_MIGRATE_PARAM_MIGRATE_DISKS, + val) < 0) { +VIR_FREE(opt_copy); +VIR_FREE(val); +goto save_error; +} + +VIR_FREE(opt_copy); +VIR_FREE(val); +} + if (vshCommandOptStringReq(ctl, cmd, "xml", &opt) < 0) goto out; if (opt) { -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] daemon: logging: Suppress logging of VIR_ERR_NO_DOMAIN_METADATA
Similarly to other error codes that notify the user that the object does not exist lower the priority of VIR_ERR_NO_DOMAIN_METADATA to VIR_LOG_DEBUG when writing the log entry. --- daemon/libvirtd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 107b88d..3e7f87c 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -320,6 +320,7 @@ static int daemonErrorLogFilter(virErrorPtr err, int priority) case VIR_ERR_NO_SECRET: case VIR_ERR_NO_DOMAIN_SNAPSHOT: case VIR_ERR_OPERATION_INVALID: +case VIR_ERR_NO_DOMAIN_METADATA: return VIR_LOG_DEBUG; } -- 2.3.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libxl: support VNC passwd
On Fri, May 08, 2015 at 04:44:14PM -0600, Jim Fehlig wrote: While implementing support for SPICE, noticed VNC passwd was never s/, n/, I n/ ? copied to libxl_device_vfb's vnc.passwd field. ACK Signed-off-by: Jim Fehlig --- src/libxl/libxl_conf.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index fccada5..35b1d04 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1283,6 +1283,8 @@ libxlMakeVfb(virPortAllocatorPtr graphicsports, if (VIR_STRDUP(x_vfb->vnc.listen, listenAddr) < 0) return -1; } +if (VIR_STRDUP(x_vfb->vnc.passwd, l_vfb->data.vnc.auth.passwd) < 0) +return -1; if (VIR_STRDUP(x_vfb->keymap, l_vfb->data.vnc.keymap) < 0) return -1; break; -- 1.8.4.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] remove 2 unused functions
On Tue, May 12, 2015 at 07:26:11PM +0800, Zhang Bo wrote: > From: YueWenyuan > > remove unused functions virTimeFieldsNow() and virTimeFieldsNowRaw() Removing unused functions like this is really a non-goal. They are general utility functions in libvirt that may or may not be used at a single point in time, but we do not remove them just because they happen to not currently be used. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] remove unused function virTimeFieldsNowRaw
From: YueWenyuan remove unused function virTimeFieldsNowRaw() in src/libvirt_private.syms, src/util/virtime.c and src/util/virtime.h Signed-off-by: YueWenyuan Signed-off-by: Zhang Bo --- src/libvirt_private.syms | 1 - src/util/virtime.c | 22 -- src/util/virtime.h | 2 -- 3 files changed, 25 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 43b041f..800fe95 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2230,7 +2230,6 @@ virThreadPoolSendJob; # util/virtime.h -virTimeFieldsNowRaw; virTimeFieldsThen; virTimeLocalOffsetFromUTC; virTimeMillisNow; diff --git a/src/util/virtime.c b/src/util/virtime.c index 7013caa..f127166 100644 --- a/src/util/virtime.c +++ b/src/util/virtime.c @@ -78,28 +78,6 @@ int virTimeMillisNowRaw(unsigned long long *now) } -/** - * virTimeFieldsNowRaw: - * @fields: filled with current time fields - * - * Retrieves the current time, in broken-down field format. - * The time is always in UTC. - * - * Returns 0 on success, -1 on error with errno set - */ -int virTimeFieldsNowRaw(struct tm *fields) -{ -unsigned long long now; - -if (virTimeMillisNowRaw(&now) < 0) -return -1; - -virTimeFieldsThen(now, fields); - -return 0; -} - - #define SECS_PER_HOUR (60 * 60) #define SECS_PER_DAY(SECS_PER_HOUR * 24) #define DIV(a, b) ((a) / (b) - ((a) % (b) < 0)) diff --git a/src/util/virtime.h b/src/util/virtime.h index 86dddc1..12c1303 100644 --- a/src/util/virtime.h +++ b/src/util/virtime.h @@ -44,8 +44,6 @@ void virTimeFieldsThen(unsigned long long when, struct tm *fields) * errno on failure */ int virTimeMillisNowRaw(unsigned long long *now) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; -int virTimeFieldsNowRaw(struct tm *fields) -ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; int virTimeStringNowRaw(char *buf) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; int virTimeStringThenRaw(unsigned long long when, char *buf) -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] remove unused function virTimeFieldsNow
From: YueWenyuan remove unused function virTimeFieldsNow() in src/libvirt_private.syms, src/util/virtime.c and src/util/virtime.h Signed-off-by: YueWenyuan Signed-off-by: Zhang Bo --- src/libvirt_private.syms | 1 - src/util/virtime.c | 21 - src/util/virtime.h | 2 -- 3 files changed, 24 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8c50ea2..43b041f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2230,7 +2230,6 @@ virThreadPoolSendJob; # util/virtime.h -virTimeFieldsNow; virTimeFieldsNowRaw; virTimeFieldsThen; virTimeLocalOffsetFromUTC; diff --git a/src/util/virtime.c b/src/util/virtime.c index 9d365d5..7013caa 100644 --- a/src/util/virtime.c +++ b/src/util/virtime.c @@ -242,27 +242,6 @@ int virTimeMillisNow(unsigned long long *now) /** - * virTimeFieldsNowRaw: - * @fields: filled with current time fields - * - * Retrieves the current time, in broken-down field format. - * The time is always in UTC. - * - * Returns 0 on success, -1 on error with errno reported - */ -int virTimeFieldsNow(struct tm *fields) -{ -unsigned long long now; - -if (virTimeMillisNow(&now) < 0) -return -1; - -virTimeFieldsThen(now, fields); -return 0; -} - - -/** * virTimeStringNow: * * Creates a string containing a formatted timestamp diff --git a/src/util/virtime.h b/src/util/virtime.h index 8ebad38..86dddc1 100644 --- a/src/util/virtime.h +++ b/src/util/virtime.h @@ -56,8 +56,6 @@ int virTimeStringThenRaw(unsigned long long when, char *buf) */ int virTimeMillisNow(unsigned long long *now) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; -int virTimeFieldsNow(struct tm *fields) -ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; char *virTimeStringNow(void); char *virTimeStringThen(unsigned long long when); -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] remove 2 unused functions
From: YueWenyuan remove unused functions virTimeFieldsNow() and virTimeFieldsNowRaw() YueWenyuan (2): remove unused function virTimeFieldsNow remove unused function virTimeFieldsNowRaw src/libvirt_private.syms | 2 -- src/util/virtime.c | 43 --- src/util/virtime.h | 4 3 files changed, 49 deletions(-) -- 1.7.12.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] XML: escape strings where we should do it
On Tue, May 12, 2015 at 10:07:21AM +0200, Ján Tomko wrote: > On Mon, May 11, 2015 at 06:17:26PM +0200, Pavel Hrdina wrote: > > There is a lot of places, were it's pretty ease for user to enter some > > characters that we need to escape to create a valid XML description. > > > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1197580 > > > > Signed-off-by: Pavel Hrdina > > --- > > > > Previous version: > > https://www.redhat.com/archives/libvir-list/2015-May/msg00110.html > > > > I've went through all the usage of virBufferAsprintf and hopefully I didn't > > miss > > any wrong usage. > > > > src/conf/capabilities.c| 6 ++-- > > src/conf/cpu_conf.c| 6 ++-- > > src/conf/domain_capabilities.c | 2 +- > > src/conf/domain_conf.c | 63 > > +- > > src/conf/network_conf.c| 17 ++-- > > src/conf/node_device_conf.c| 4 +-- > > 6 files changed, 49 insertions(+), 49 deletions(-) > > > > diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c > > index c43bfb3..6bd06e5 100644 > > --- a/src/conf/capabilities.c > > +++ b/src/conf/capabilities.c > > @@ -682,9 +682,9 @@ virCapabilitiesDomainDataLookupInternal(virCapsPtr caps, > > virBufferAsprintf(&buf, "domaintype=%s ", > >virDomainVirtTypeToString(domaintype)); > > if (emulator) > > -virBufferAsprintf(&buf, "emulator=%s ", emulator); > > +virBufferEscapeString(&buf, "emulator=%s ", emulator); > > virBufferEscapeString is a no-op if the string is NULL, the if is not > necessary. > > > if (machinetype) > > -virBufferAsprintf(&buf, "machine=%s ", machinetype); > > +virBufferEscapeString(&buf, "machine=%s ", machinetype); > > if (virBufferCurrentContent(&buf) && > > !virBufferCurrentContent(&buf)[0]) > > virBufferAsprintf(&buf, "%s", _("any configuration")); > > @@ -903,7 +903,7 @@ virCapabilitiesFormatXML(virCapsPtr caps) > > virBufferAdjustIndent(&buf, 2); > > for (i = 0; i < caps->host.nmigrateTrans; i++) { > > virBufferAsprintf(&buf, > > "%s\n", > > - caps->host.migrateTrans[i]); > > + caps->host.migrateTrans[i]); > > Unrelated whitespace change. > > > } > > virBufferAdjustIndent(&buf, -2); > > virBufferAddLit(&buf, "\n"); > > diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c > > index e959ecc..1ba1d82 100644 > > --- a/src/conf/cpu_conf.c > > +++ b/src/conf/cpu_conf.c > > @@ -544,17 +544,17 @@ virCPUDefFormatBuf(virBufferPtr buf, > > } > > virBufferAsprintf(buf, " fallback='%s'", fallback); > > if (def->vendor_id) > > -virBufferAsprintf(buf, " vendor_id='%s'", def->vendor_id); > > +virBufferEscapeString(buf, " vendor_id='%s'", > > def->vendor_id); > > } > > if (formatModel && def->model) { > > -virBufferAsprintf(buf, ">%s\n", def->model); > > +virBufferEscapeString(buf, ">%s\n", def->model); > > We need to keep check for non-NULL string in the if here. > > > } else { > > virBufferAddLit(buf, "/>\n"); > > } > > } > > > > if (formatModel && def->vendor) > > -virBufferAsprintf(buf, "%s\n", def->vendor); > > +virBufferEscapeString(buf, "%s\n", def->vendor); > > > > But not here. > > > if (def->sockets && def->cores && def->threads) { > > virBufferAddLit(buf, " > diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c > > index 7c59912..0e32f52 100644 > > --- a/src/conf/domain_capabilities.c > > +++ b/src/conf/domain_capabilities.c > > @@ -272,7 +272,7 @@ virDomainCapsFormatInternal(virBufferPtr buf, > > virBufferAddLit(buf, "\n"); > > virBufferAdjustIndent(buf, 2); > > > > -virBufferAsprintf(buf, "%s\n", caps->path); > > +virBufferEscapeString(buf, "%s\n", caps->path); > > virBufferAsprintf(buf, "%s\n", virttype_str); > > virBufferAsprintf(buf, "%s\n", caps->machine); > > virBufferAsprintf(buf, "%s\n", arch_str); > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > > index e3e0f63..4c3f46f 100644 > > --- a/src/conf/domain_conf.c > > +++ b/src/conf/domain_conf.c > > @@ -3724,7 +3724,7 @@ virDomainDeviceInfoFormat(virBufferPtr buf, > > virBufferAsprintf(buf, " bar='%s'", rombar); > > } > > if (info->romfile) > > -virBufferAsprintf(buf, " file='%s'", info->romfile); > > +virBufferEscapeString(buf, " file='%s'", info->romfile); > > virBufferAddLit(buf, "/>\n"); > > } > > > > @@ -17712,7 +17712,7 @@ virSecurityDeviceLabelDefFormat(virBufferPtr buf, > > virBufferAddLit(buf, " > > > if (def->model) > > -virBufferAsprint
Re: [libvirt] cpuset / numa and qemu in TCG mode
On Tue, May 12, 2015 at 09:51:24AM +0100, Daniel P. Berrange wrote: > I think this is something distros should really fix in their libvirt > packages - backporting this kind of fix is exactly the kind job of > maintainers should be doing. > > Particularly since it only affects TCG mode, I don't think Nova really > should try to workaround it Thanks Daniel. Yours Tony. pgpilwiUkTc0B.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] cpuset / numa and qemu in TCG mode
On Tue, May 12, 2015 at 11:14:09AM +0200, Martin Kletzander wrote: > 1.2.13 has the commit already in the release and 1.2.12-maint has it > as a first back-port right after release. The problem is that there > was no maintenance release of 1.2.12 yet. Maybe they would use > 1.2.12.1 if it existed. Perhaps. At this point we're well and truly in realms I can't influence. I thank you for you help keeping me on track here. I think by and large the issue is preety much solved (and was before I started this thread a couple of days ago. > I Cc'd Guido as an upstream debian maintainer, maybe he'll have some > insights. @Guido: would it help if we created a maintenance release > from the v1.2.12-maint branch? Or is the only thing missing the fact > that the launchpad bug is not moved to libvirt? I just added: --- I was pointed at the v1.2.12-maint head in the libvirt git which contains this fix already. http://libvirt.org/git/?p=libvirt.git;a=shortlog;h=refs/heads/v1.2.12-maint I suggest we close the nova issue with won't fix and get the correctly backported patch into the libvirt package. --- https://bugs.launchpad.net/ubuntu/+source/nova/+bug/1439280/comments/41 It's up to the Ubuntu team now. Yours Tony. pgpiX3JP0_BnX.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] cpuset / numa and qemu in TCG mode
On Tue, May 12, 2015 at 05:27:34PM +1000, Tony Breeds wrote: On Mon, May 11, 2015 at 01:14:58PM +0200, Martin Kletzander wrote: Determining this by version might not be reliable, but more importantly working around bug in underlying software is something that shouldn't be done at all IMHO. Let the maintainers backport whatever needs to be done. I agree with you in an ideal world but there are times when we do need to add work arounds in $project_x to work around issues in $project_y. >Ther nova side will be pretty easy regardless. > >I'd say the best solution would be to back port the 'fix' but that seems like a >lot of effort given the number of distros and libvirt versions potentiall >involved. > If you want the fix to be distro-agnostic, there's nothing easier than back-porting the fix into our upstream maintenance branches. Those should make the life of distro maintainers easy (although it looks like not many distros use it). And this is part of the problem. If I understand correctly Ubuntu cloud-archive is using libvirt 1.2.12 which is *NOT* a maintenance release so that leaves us with doing an additional backport to 1.2.12 and getting the cloud-archive team to take it[1] or Adding a hack to nova. And that's just Ubuntu It's hard to say for sure that some vendor isn't running libvirt 1.2.12 also. Having said that I'm not sure which commit(s) are those that need to be back-ported. Having known your libvirt version, it shouldn't be too hard looking for the differences and finding the right commit. When back-porting request is made on the list, it is usually acted upon. If you can't find the exact commit, let me know and I'll do my best to help. So a git bisect points at: --- commit a103bb105c0c189c3973311ff1826972b5bc6ad6 Author: Daniel P. Berrange Date: Tue Feb 10 15:59:57 2015 + qemu: fix setting of VM CPU affinity with TCG --- A small amount of reading implies to me that we'd be looking at backporting a103bb105c0c189c3973311ff1826972b5bc6ad6 to any maintenance branch that contains b07f3d821dfb11a118ee75ea275fd6ab737d9500. Which I think is 1.2.13 only, but I could be wrong. 1.2.13 has the commit already in the release and 1.2.12-maint has it as a first back-port right after release. The problem is that there was no maintenance release of 1.2.12 yet. Maybe they would use 1.2.12.1 if it existed. I Cc'd Guido as an upstream debian maintainer, maybe he'll have some insights. @Guido: would it help if we created a maintenance release from the v1.2.12-maint branch? Or is the only thing missing the fact that the launchpad bug is not moved to libvirt? If you don't beat me to it I'll request that backport to 1.2.13 *and* ask the Ubuntu guys to take it as well. I have to admit I'm still in 2 minds on the nova side. Adding a wart to the libvirt driver for this specific bug for distros/vendors that are using 1.2.12 seems a bit gross but Daniel what do you think? Yours Tony. signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] time: event poll may become un-triggerable after changing system clock.
On Tue, May 12, 2015 at 04:14:37PM +0800, zhang bo wrote: > event poll may become un-triggerable after changing system clock. > > The steps to reproduce the problem: > 1 run event-test > 1 define and start a domain with name vm1. > 2 destroy vm1 > 3 change system time to 1 hour before when timer.expiresAt has been set in > virEventPollUpdateTimeout > (and before virEventPollDispatchTimeouts()). > 4 event-test will recive no message until 1 hour later. > > > The reasons for the problem is : > 1 The value of timer.expiresAt is set by virTimeMillisNowRaw. > virTimeMillisNowRaw is effectable by settimeofday(), > bacause it uses CLOCK_REALTIME to get time. > 2 If we change the system time to a time long before now, after that > timer.expiresAt has been set. timer.expiresAt > is not affected, while virEventPollDispatchTimeouts is. > Suppose it's now May 12th, and we set it to 10th, then the expiresAt is > 12th, and the time virEventPollDispatchTimeouts > got is 10th. > if (eventLoop.timeouts[i].expiresAt <= (now+20)) { // expiresAt will > not be less than now until 2 days later. > > > > *Solution(not good enough)*: > 1 change the clock mode in virTimeMillisNowRaw from REALTIME to MONOTONIC, > which would not be affected by > settimeofday(). > 2 add the time got from clock_gettime(*MONOTONIC*) with the system-start-time > from epoch, making it equal to the value got from REALTIME. > 3 As that the timestamp of the log message should follow system time, so we > keep it to REALTIME as before. > > However, there's still problems: > 1 pthread_cond_wait() gets time with REALTIME mode. When we change system > time, pthread_cond_wait() may still be affected. > > So, Is there any other better solution? thanks in advance. Simply don't change the system time by massive deltas. Libvirt is not going to be the only app to be affected. As you mention it is going to hit the pthread_cond_wait() call which will likely affect pretty much every single non-trivial process running on the system. I'd expect other apps have much the same problem with calculating poll sleeps too. If you need to massively change the system time this should be done at single user mode, or do a reboot. Once a system is running it should be kept synced with NTPD which will only ever change system time in very small increments and so once cause thsi problem. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Document that virNodeGetInfo can return mhz == 0.
On Mon, May 11, 2015 at 09:30:06PM +0100, Richard W.M. Jones wrote: > On the s/390x architecture, libvirt may already return 0 in the > node_info->mhz field (see src/nodeinfo.c:linuxNodeInfoCPUPopulate). > > We may also want to return this on aarch64 in future, because > calculating the proper value requires SMBIOS, which is not available > on non-server-class systems (specifically on systems which don't > adhere to the SBSA standard). > > Therefore this change documents the existing behaviour and provides a > valid path for aarch64. > > Signed-off-by: Richard W.M. Jones > Bug-URL: https://bugzilla.redhat.com/1206353 > --- > include/libvirt/libvirt-host.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) ACK Even on x86 this can be fairly meaningless due to CPU freq scaling in the OS Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] cpuset / numa and qemu in TCG mode
On Tue, May 12, 2015 at 05:27:34PM +1000, Tony Breeds wrote: > On Mon, May 11, 2015 at 01:14:58PM +0200, Martin Kletzander wrote: > > > Determining this by version might not be reliable, but more > > importantly working around bug in underlying software is something > > that shouldn't be done at all IMHO. Let the maintainers backport > > whatever needs to be done. > > I agree with you in an ideal world but there are times when we do need > to add work arounds in $project_x to work around issues in $project_y. > > > >Ther nova side will be pretty easy regardless. > > > > > >I'd say the best solution would be to back port the 'fix' but that seems > > >like a > > >lot of effort given the number of distros and libvirt versions potentiall > > >involved. > > > > > > > If you want the fix to be distro-agnostic, there's nothing easier than > > back-porting the fix into our upstream maintenance branches. Those > > should make the life of distro maintainers easy (although it looks > > like not many distros use it). > > And this is part of the problem. If I understand correctly Ubuntu > cloud-archive > is using libvirt 1.2.12 which is *NOT* a maintenance release so that leaves us > with doing an additional backport to 1.2.12 and getting the cloud-archive team > to take it[1] or Adding a hack to nova. And that's just Ubuntu It's hard to > say for sure that some vendor isn't running libvirt 1.2.12 also. > > > Having said that I'm not sure which commit(s) are those that need to > > be back-ported. Having known your libvirt version, it shouldn't be > > too hard looking for the differences and finding the right commit. > > When back-porting request is made on the list, it is usually acted > > upon. If you can't find the exact commit, let me know and I'll do my > > best to help. > > So a git bisect points at: > --- > commit a103bb105c0c189c3973311ff1826972b5bc6ad6 > Author: Daniel P. Berrange > Date: Tue Feb 10 15:59:57 2015 + > > qemu: fix setting of VM CPU affinity with TCG > --- > > A small amount of reading implies to me that we'd be looking at backporting > a103bb105c0c189c3973311ff1826972b5bc6ad6 to any maintenance branch that > contains > b07f3d821dfb11a118ee75ea275fd6ab737d9500. Which I think is 1.2.13 only, but I > could be wrong. > > If you don't beat me to it I'll request that backport to 1.2.13 *and* ask the > Ubuntu guys to take it as well. > > I have to admit I'm still in 2 minds on the nova side. Adding a wart to the > libvirt driver for this specific bug for distros/vendors that are using 1.2.12 > seems a bit gross but I think this is something distros should really fix in their libvirt packages - backporting this kind of fix is exactly the kind job of maintainers should be doing. Particularly since it only affects TCG mode, I don't think Nova really should try to workaround it Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] time: event poll may become un-triggerable after changing system clock.
event poll may become un-triggerable after changing system clock. The steps to reproduce the problem: 1 run event-test 1 define and start a domain with name vm1. 2 destroy vm1 3 change system time to 1 hour before when timer.expiresAt has been set in virEventPollUpdateTimeout (and before virEventPollDispatchTimeouts()). 4 event-test will recive no message until 1 hour later. The reasons for the problem is : 1 The value of timer.expiresAt is set by virTimeMillisNowRaw. virTimeMillisNowRaw is effectable by settimeofday(), bacause it uses CLOCK_REALTIME to get time. 2 If we change the system time to a time long before now, after that timer.expiresAt has been set. timer.expiresAt is not affected, while virEventPollDispatchTimeouts is. Suppose it's now May 12th, and we set it to 10th, then the expiresAt is 12th, and the time virEventPollDispatchTimeouts got is 10th. if (eventLoop.timeouts[i].expiresAt <= (now+20)) { // expiresAt will not be less than now until 2 days later. *Solution(not good enough)*: 1 change the clock mode in virTimeMillisNowRaw from REALTIME to MONOTONIC, which would not be affected by settimeofday(). 2 add the time got from clock_gettime(*MONOTONIC*) with the system-start-time from epoch, making it equal to the value got from REALTIME. 3 As that the timestamp of the log message should follow system time, so we keep it to REALTIME as before. However, there's still problems: 1 pthread_cond_wait() gets time with REALTIME mode. When we change system time, pthread_cond_wait() may still be affected. So, Is there any other better solution? thanks in advance. oscar -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] XML: escape strings where we should do it
On Mon, May 11, 2015 at 06:17:26PM +0200, Pavel Hrdina wrote: > There is a lot of places, were it's pretty ease for user to enter some > characters that we need to escape to create a valid XML description. > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1197580 > > Signed-off-by: Pavel Hrdina > --- > > Previous version: > https://www.redhat.com/archives/libvir-list/2015-May/msg00110.html > > I've went through all the usage of virBufferAsprintf and hopefully I didn't > miss > any wrong usage. > > src/conf/capabilities.c| 6 ++-- > src/conf/cpu_conf.c| 6 ++-- > src/conf/domain_capabilities.c | 2 +- > src/conf/domain_conf.c | 63 > +- > src/conf/network_conf.c| 17 ++-- > src/conf/node_device_conf.c| 4 +-- > 6 files changed, 49 insertions(+), 49 deletions(-) > > diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c > index c43bfb3..6bd06e5 100644 > --- a/src/conf/capabilities.c > +++ b/src/conf/capabilities.c > @@ -682,9 +682,9 @@ virCapabilitiesDomainDataLookupInternal(virCapsPtr caps, > virBufferAsprintf(&buf, "domaintype=%s ", >virDomainVirtTypeToString(domaintype)); > if (emulator) > -virBufferAsprintf(&buf, "emulator=%s ", emulator); > +virBufferEscapeString(&buf, "emulator=%s ", emulator); virBufferEscapeString is a no-op if the string is NULL, the if is not necessary. > if (machinetype) > -virBufferAsprintf(&buf, "machine=%s ", machinetype); > +virBufferEscapeString(&buf, "machine=%s ", machinetype); > if (virBufferCurrentContent(&buf) && > !virBufferCurrentContent(&buf)[0]) > virBufferAsprintf(&buf, "%s", _("any configuration")); > @@ -903,7 +903,7 @@ virCapabilitiesFormatXML(virCapsPtr caps) > virBufferAdjustIndent(&buf, 2); > for (i = 0; i < caps->host.nmigrateTrans; i++) { > virBufferAsprintf(&buf, > "%s\n", > - caps->host.migrateTrans[i]); > + caps->host.migrateTrans[i]); Unrelated whitespace change. > } > virBufferAdjustIndent(&buf, -2); > virBufferAddLit(&buf, "\n"); > diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c > index e959ecc..1ba1d82 100644 > --- a/src/conf/cpu_conf.c > +++ b/src/conf/cpu_conf.c > @@ -544,17 +544,17 @@ virCPUDefFormatBuf(virBufferPtr buf, > } > virBufferAsprintf(buf, " fallback='%s'", fallback); > if (def->vendor_id) > -virBufferAsprintf(buf, " vendor_id='%s'", def->vendor_id); > +virBufferEscapeString(buf, " vendor_id='%s'", > def->vendor_id); > } > if (formatModel && def->model) { > -virBufferAsprintf(buf, ">%s\n", def->model); > +virBufferEscapeString(buf, ">%s\n", def->model); We need to keep check for non-NULL string in the if here. > } else { > virBufferAddLit(buf, "/>\n"); > } > } > > if (formatModel && def->vendor) > -virBufferAsprintf(buf, "%s\n", def->vendor); > +virBufferEscapeString(buf, "%s\n", def->vendor); > But not here. > if (def->sockets && def->cores && def->threads) { > virBufferAddLit(buf, " diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c > index 7c59912..0e32f52 100644 > --- a/src/conf/domain_capabilities.c > +++ b/src/conf/domain_capabilities.c > @@ -272,7 +272,7 @@ virDomainCapsFormatInternal(virBufferPtr buf, > virBufferAddLit(buf, "\n"); > virBufferAdjustIndent(buf, 2); > > -virBufferAsprintf(buf, "%s\n", caps->path); > +virBufferEscapeString(buf, "%s\n", caps->path); > virBufferAsprintf(buf, "%s\n", virttype_str); > virBufferAsprintf(buf, "%s\n", caps->machine); > virBufferAsprintf(buf, "%s\n", arch_str); > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index e3e0f63..4c3f46f 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -3724,7 +3724,7 @@ virDomainDeviceInfoFormat(virBufferPtr buf, > virBufferAsprintf(buf, " bar='%s'", rombar); > } > if (info->romfile) > -virBufferAsprintf(buf, " file='%s'", info->romfile); > +virBufferEscapeString(buf, " file='%s'", info->romfile); > virBufferAddLit(buf, "/>\n"); > } > > @@ -17712,7 +17712,7 @@ virSecurityDeviceLabelDefFormat(virBufferPtr buf, > virBufferAddLit(buf, " > if (def->model) > -virBufferAsprintf(buf, " model='%s'", def->model); > +virBufferEscapeString(buf, " model='%s'", def->model); > > if (def->labelskip) > virBufferAddLit(buf, " labelskip='yes'"); > @@ -19246,50 +19246,51 @@ virDomainChrSourceDefFormat(virBufferPtr buf, > break; > > case VIR_DOMA
Re: [libvirt] cpuset / numa and qemu in TCG mode
On Mon, May 11, 2015 at 01:14:58PM +0200, Martin Kletzander wrote: > Determining this by version might not be reliable, but more > importantly working around bug in underlying software is something > that shouldn't be done at all IMHO. Let the maintainers backport > whatever needs to be done. I agree with you in an ideal world but there are times when we do need to add work arounds in $project_x to work around issues in $project_y. > >Ther nova side will be pretty easy regardless. > > > >I'd say the best solution would be to back port the 'fix' but that seems > >like a > >lot of effort given the number of distros and libvirt versions potentiall > >involved. > > > > If you want the fix to be distro-agnostic, there's nothing easier than > back-porting the fix into our upstream maintenance branches. Those > should make the life of distro maintainers easy (although it looks > like not many distros use it). And this is part of the problem. If I understand correctly Ubuntu cloud-archive is using libvirt 1.2.12 which is *NOT* a maintenance release so that leaves us with doing an additional backport to 1.2.12 and getting the cloud-archive team to take it[1] or Adding a hack to nova. And that's just Ubuntu It's hard to say for sure that some vendor isn't running libvirt 1.2.12 also. > Having said that I'm not sure which commit(s) are those that need to > be back-ported. Having known your libvirt version, it shouldn't be > too hard looking for the differences and finding the right commit. > When back-porting request is made on the list, it is usually acted > upon. If you can't find the exact commit, let me know and I'll do my > best to help. So a git bisect points at: --- commit a103bb105c0c189c3973311ff1826972b5bc6ad6 Author: Daniel P. Berrange Date: Tue Feb 10 15:59:57 2015 + qemu: fix setting of VM CPU affinity with TCG --- A small amount of reading implies to me that we'd be looking at backporting a103bb105c0c189c3973311ff1826972b5bc6ad6 to any maintenance branch that contains b07f3d821dfb11a118ee75ea275fd6ab737d9500. Which I think is 1.2.13 only, but I could be wrong. If you don't beat me to it I'll request that backport to 1.2.13 *and* ask the Ubuntu guys to take it as well. I have to admit I'm still in 2 minds on the nova side. Adding a wart to the libvirt driver for this specific bug for distros/vendors that are using 1.2.12 seems a bit gross but Daniel what do you think? Yours Tony. pgp4vOCe9H2Hq.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list