[libvirt] [PATCH] qemu: clear useless code

2015-05-12 Thread zhang bo
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

2015-05-12 Thread lhuang


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]

2015-05-12 Thread lhuang


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

2015-05-12 Thread Chen Fan
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

2015-05-12 Thread Chen Fan
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

2015-05-12 Thread Chen Fan
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

2015-05-12 Thread Chen Fan
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

2015-05-12 Thread Chen Fan
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

2015-05-12 Thread Chen Fan
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

2015-05-12 Thread Chen Fan
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.

2015-05-12 Thread zhang bo
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

2015-05-12 Thread Jim Fehlig
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

2015-05-12 Thread Jim Fehlig
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

2015-05-12 Thread John Ferlan
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

2015-05-12 Thread John Ferlan


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

2015-05-12 Thread Eric Blake
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

2015-05-12 Thread Eric Blake
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

2015-05-12 Thread Pavel Hrdina
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

2015-05-12 Thread Guido Günther
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

2015-05-12 Thread Michal Privoznik
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

2015-05-12 Thread Michal Privoznik
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

2015-05-12 Thread Michal Privoznik
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

2015-05-12 Thread Peter Krempa
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

2015-05-12 Thread Tony Krowiak

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

2015-05-12 Thread Laine Stump
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

2015-05-12 Thread Michal Privoznik
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

2015-05-12 Thread Michal Privoznik
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

2015-05-12 Thread Peter Krempa
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

2015-05-12 Thread Michal Privoznik
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
 
+
+  LENOVO
+  20BE0061MC
+  0B98401 Pro
+  W1KS427111E
+  Motherboard
+
   
   ...
 
@@ -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;
+

[libvirt] [PATCH 1/3] virSysinfoDef: Exempt BIOS variables

2015-05-12 Thread Michal Privoznik
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

2015-05-12 Thread Michal Privoznik
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

2015-05-12 Thread Michal Privoznik
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.

2015-05-12 Thread Peter Krempa
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

2015-05-12 Thread Luyao Huang
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

2015-05-12 Thread Peter Krempa
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

2015-05-12 Thread Jiri Denemark
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

2015-05-12 Thread Martin Kletzander

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

2015-05-12 Thread Daniel P. Berrange
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

2015-05-12 Thread Jiri Denemark
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

2015-05-12 Thread Jiri Denemark
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

2015-05-12 Thread Jiri Denemark
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

2015-05-12 Thread Jiri Denemark
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

2015-05-12 Thread Peter Krempa
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

2015-05-12 Thread Pavel Boldin
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

2015-05-12 Thread Pavel Boldin
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

2015-05-12 Thread Pavel Boldin
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

2015-05-12 Thread Pavel Boldin
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

2015-05-12 Thread Ján Tomko
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

2015-05-12 Thread Pavel Boldin
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

2015-05-12 Thread Pavel Boldin
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

2015-05-12 Thread Peter Krempa
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

2015-05-12 Thread Martin Kletzander

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

2015-05-12 Thread Daniel P. Berrange
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

2015-05-12 Thread Zhang Bo
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

2015-05-12 Thread Zhang Bo
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

2015-05-12 Thread Zhang Bo
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

2015-05-12 Thread Pavel Hrdina
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

2015-05-12 Thread Tony Breeds
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

2015-05-12 Thread Tony Breeds
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

2015-05-12 Thread Martin Kletzander

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.

2015-05-12 Thread Daniel P. Berrange
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.

2015-05-12 Thread Daniel P. Berrange
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

2015-05-12 Thread Daniel P. Berrange
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.

2015-05-12 Thread zhang bo
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

2015-05-12 Thread Ján Tomko
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

2015-05-12 Thread Tony Breeds
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