[libvirt] [PATCH v2 0/1] qemu: monitor: Add memory balloon support for virtio-ccw

[Boris Fiuczynski]

v2:
 * corrected historic spelling error in comment
 * added return code checking of the first qemuMonitorJSONFindLinkPath call to
prevent the second call of qemuMonitorJSONFindLinkPath on "error bail out"
return code.

A comment on this version: I find that even so this version is technically more
correct the simplicity and perceivability have suffered profoundly. I am not 
sure
that after the recent rework by which the init function protects itself from
being fully run a second time during a run of the domain it is necessary to
prevent a single second call returning "error bail out".

Boris Fiuczynski (1):
  qemu: monitor: Add memory balloon support for virtio-ccw

 src/qemu/qemu_monitor.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

-- 
2.3.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 06/22] Pass domain object to private data formatter/parser

[Peter Krempa]

On Tue, Jun 02, 2015 at 14:34:11 +0200, Jiri Denemark wrote:
> So that they can format private data (e.g., disk private data) stored
> elsewhere in the domain object.
> 
> Signed-off-by: Jiri Denemark 
> ---
> 

ACK,

Peter


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 1/1] qemu: monitor: Add memory balloon support for virtio-ccw

[Boris Fiuczynski]

The search for the memory balloon driver object is extended by a
second known name "virtio-balloon-ccw" in support for virtio-ccw.

Signed-off-by: Boris Fiuczynski 
Reviewed-by: Daniel Hansel 
Reviewed-by: Eric Farman 
Reviewed-by: Stefan Zimmermann 
---
 src/qemu/qemu_monitor.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 33600f0..6f2f4a9 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1069,9 +1069,9 @@ qemuMonitorSetOptions(qemuMonitorPtr mon, virJSONValuePtr 
options)
 
 
 /**
- * Search the qom objects for the balloon driver object by it's known name
- * of "virtio-balloon-pci".  The entry for the driver will be found by using
- * function "qemuMonitorFindObjectPath".
+ * Search the qom objects for the balloon driver object by its known names
+ * of "virtio-balloon-pci" or "virtio-balloon-ccw". The entry for the driver
+ * will be found by using function "qemuMonitorJSONFindLinkPath".
  *
  * Once found, check the entry to ensure it has the correct property listed.
  * If it does not, then obtaining statistics from QEMU will not be possible.
@@ -1081,6 +1081,7 @@ static void
 qemuMonitorInitBalloonObjectPath(qemuMonitorPtr mon)
 {
 ssize_t i, nprops = 0;
+int flp_ret = 0;
 char *path = NULL;
 qemuMonitorJSONListPathPtr *bprops = NULL;
 
@@ -1093,8 +1094,14 @@ qemuMonitorInitBalloonObjectPath(qemuMonitorPtr mon)
 }
 mon->ballooninit = true;
 
-if (qemuMonitorJSONFindLinkPath(mon, "virtio-balloon-pci", &path) < 0)
+flp_ret = qemuMonitorJSONFindLinkPath(mon, "virtio-balloon-pci", &path);
+if (flp_ret == -2) {
+/* pci object was not found retry search for ccw object */
+if (qemuMonitorJSONFindLinkPath(mon, "virtio-balloon-ccw", &path) < 0)
+return;
+} else if (flp_ret < 0) {
 return;
+}
 
 nprops = qemuMonitorJSONGetObjectListPaths(mon, path, &bprops);
 if (nprops < 0)
-- 
2.3.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 07/22] qemu: Make qemuMigrationCancelDriveMirror usable without async job

[Peter Krempa]

On Tue, Jun 02, 2015 at 14:34:12 +0200, Jiri Denemark wrote:
> We don't have an async job when reconnecting to existing domains after
> libvirtd restart.
> 
> Signed-off-by: Jiri Denemark 
> ---

ACK,

Peter


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] computing the correct rom for seabios to recognize SR-IOV

[Ido Barkan]

Thanks Laine.

I was told by ghoff...@redhat.com [cc'ed] that:
"""
"ipxe-roms-qemu.rpm" carries only the roms for the hardware emulated by qemu, 
and qemu 
has a rpm dependency on it so this is installed on every virt host.
"ipxe-roms.rpm" has everything else, but isn't installed by default.

And that Naming convention used by ipxe is "${vendorid}${deviceid}.rom" for the 
filename.
"8086" is vendor intel, "10ca" is the the vf nic device.
"""

So it's VDSM responsibility to depend on ipxe-roms.rpm in order to pass-through 
physical 
hardware, and it seems that there is a standard way of computing a proper boot 
rom for a device.

Since every user of hostdev having this this use case will have to do the same 
computation,
I was hoping that libvirt could do it for us.

Thanks,
Ido

- Original Message -
From: "Laine Stump" 
To: "Ido Barkan" , libvir-list@redhat.com
Cc: "Alona Kaplan" 
Sent: Tuesday, June 9, 2015 5:20:07 PM
Subject: Re: [libvirt] computing the correct rom for seabios to recognize SR-IOV

On 06/09/2015 06:49 AM, Ido Barkan wrote:
> Hi,
> I am a VDSM developer at Ovirt.
>
> Recently, we opened Bug 1224954 - seabios does not recognize a direct 
> attached nic [1]
> and discovered that in order to leverage the 
> (as explained in [2]) in the hostdev element, one must compute the correct 
> FILE 
> path by following something similar to:
>
> $ lspci -v #loof for the VF nic part
> 00:08.0 Ethernet controller: Intel Corporation 82576 Virtual Function (rev 01)
> $ grep "82576 Virtual Function" /usr/share/hwdata/pci.ids 
>10ca  82576 Virtual Function
> $ rpm -ql ipxe-roms | grep 10ca
> /usr/share/ipxe/808610ca.rom
>
> This computation must be implemented for Ovirt to support booting a VM from 
> PXE using a SRIOV VF.
>
> Is there any chance that this computation will be done automatically by 
> libvirt?

I don't think there's any kind of standard that says the proper boot rom
for a device can be found by searching for the device's id in a filename
in /usr/share/ipxe. Because the name of the bootrom file isn't fixed or
reliably predictable, I don't see how libvirt can do anything more
specific than allowing a filename to be specified, which it already does.

(for example, I have an 82576 SRIOV card on my Fedora 22 system, and a
*much* newer version of ipxe installed:

  ipxe-roms-qemu-20150407-1.gitdc795b9f.fc22.noarch

but do not have the file 808610ca.rom (or *any* file with 10ca in the
name) in /usr/share/ipxe)

So I think the name of the rom really needs to be something configurable
from within ovirt (which would then put the name in the libvirt
configuration).

(One thing this points out to me, though, is that it would be useful to
have the rom filename as a configuration option in network pools - that
would eliminate the need to specify it separately in every single domain
interface definition)

--
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 4/4] parallels: substitute parallels with vz spec file and Makefile

[Maxim Nestratov]

Since we have changed the name of the driver to vz, let's
reference it as vz everywhere.

Signed-off-by: Maxim Nestratov 
---
 configure.ac  | 26 +-
 libvirt.spec.in   | 10 +-
 mingw-libvirt.spec.in |  6 +++---
 src/Makefile.am   |  4 ++--
 src/libvirt.c |  6 +++---
 5 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/configure.ac b/configure.ac
index abf4436..81d4f07 100644
--- a/configure.ac
+++ b/configure.ac
@@ -562,10 +562,10 @@ AC_ARG_WITH([hyperv],
   [AS_HELP_STRING([--with-hyperv],
 [add Hyper-V support @<:@default=check@:>@])])
 m4_divert_text([DEFAULTS], [with_hyperv=check])
-AC_ARG_WITH([parallels],
-  [AS_HELP_STRING([--with-parallels],
-[add Parallels Cloud Server support @<:@default=check@:>@])])
-m4_divert_text([DEFAULTS], [with_parallels=check])
+AC_ARG_WITH([vz],
+  [AS_HELP_STRING([--with-vz],
+[add Virtuozzo support @<:@default=check@:>@])])
+m4_divert_text([DEFAULTS], [with_vz=check])
 AC_ARG_WITH([test],
   [AS_HELP_STRING([--with-test],
 [add test driver support @<:@default=yes@:>@])])
@@ -1083,22 +1083,22 @@ dnl Checks for the Parallels driver
 dnl
 
 
-if test "$with_parallels" = "yes" ||
-   test "$with_parallels" = "check"; then
+if test "$with_vz" = "yes" ||
+   test "$with_vz" = "check"; then
 PKG_CHECK_MODULES([PARALLELS_SDK], [parallels-sdk],
   [PARALLELS_SDK_FOUND=yes], [PARALLELS_SDK_FOUND=no])
 
-if test "$with_parallels" = "yes" && test "$PARALLELS_SDK_FOUND" = "no"; 
then
+if test "$with_vz" = "yes" && test "$PARALLELS_SDK_FOUND" = "no"; then
 AC_MSG_ERROR([Parallels Virtualization SDK is needed to build the 
Parallels driver.])
 fi
 
-with_parallels=$PARALLELS_SDK_FOUND
-if test "$with_parallels" = "yes"; then
-AC_DEFINE_UNQUOTED([WITH_PARALLELS], 1,
-   [whether Parallels driver is enabled])
+with_vz=$PARALLELS_SDK_FOUND
+if test "$with_vz" = "yes"; then
+AC_DEFINE_UNQUOTED([WITH_VZ], 1,
+   [whether vz driver is enabled])
 fi
 fi
-AM_CONDITIONAL([WITH_PARALLELS], [test "$with_parallels" = "yes"])
+AM_CONDITIONAL([WITH_VZ], [test "$with_vz" = "yes"])
 
 dnl
 dnl Checks for bhyve driver
@@ -2839,7 +2839,7 @@ AC_MSG_NOTICE([  LXC: $with_lxc])
 AC_MSG_NOTICE([ PHYP: $with_phyp])
 AC_MSG_NOTICE([  ESX: $with_esx])
 AC_MSG_NOTICE([  Hyper-V: $with_hyperv])
-AC_MSG_NOTICE([Parallels: $with_parallels])
+AC_MSG_NOTICE([   vz: $with_vz])
 LIBVIRT_DRIVER_RESULT_BHYVE
 AC_MSG_NOTICE([ Test: $with_test])
 AC_MSG_NOTICE([   Remote: $with_remote])
diff --git a/libvirt.spec.in b/libvirt.spec.in
index f16e0e5..dc2a3dc 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -97,7 +97,7 @@
 %define with_esx   0%{!?_without_esx:1}
 %define with_hyperv0%{!?_without_hyperv:1}
 %define with_xenapi0%{!?_without_xenapi:1}
-%define with_parallels 0%{!?_without_parallels:1}
+%define with_vz0%{!?_without_vz:1}
 # No test for bhyve, because it does not build on Linux
 
 # Then the secondary host drivers, which run inside libvirtd
@@ -201,7 +201,7 @@
 %define with_xenapi 0
 %define with_libxl 0
 %define with_hyperv 0
-%define with_parallels 0
+%define with_vz 0
 %endif
 
 # Fedora 17 / RHEL-7 are first where we use systemd. Although earlier
@@ -1307,8 +1307,8 @@ rm -f $PATCHLIST
 %define _without_vmware --without-vmware
 %endif
 
-%if ! %{with_parallels}
-%define _without_parallels --without-parallels
+%if ! %{with_vz}
+%define _without_vz --without-vz
 %endif
 
 %if ! %{with_polkit}
@@ -1490,7 +1490,7 @@ rm -f po/stamp-po
%{?_without_esx} \
%{?_without_hyperv} \
%{?_without_vmware} \
-   %{?_without_parallels} \
+   %{?_without_vz} \
--without-bhyve \
%{?_without_interface} \
%{?_without_network} \
diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in
index a7b3854..6f95832 100644
--- a/mingw-libvirt.spec.in
+++ b/mingw-libvirt.spec.in
@@ -13,7 +13,7 @@
 # missing libwsman, so can't build hyper-v
 %define with_hyperv0%{!?_without_hyperv:0}
 %define with_xenapi0%{!?_without_xenapi:1}
-%define with_parallels   0%{!?_without_parallels:0}
+%define with_vz0%{!?_without_vz:0}
 
 # RHEL ships ESX but not PowerHypervisor, HyperV, or libxenserver (xenapi)
 %if 0%{?rhel}
@@ -126,8 +126,8 @@ MinGW Windows libvirt virtualization library, static 
version.
 %define _without_xenapi --without-xenapi
 %endif
 
-%if ! %{with_parallels}
-%define _without_parallels --without-parallels
+%if ! %{with_vz}
+%define _without_vz --without-vz
 %endif
 
 %if 0%{?enable_autotools}
diff --git a/src/Makefile.am b/src/Makefile.am
index dc9ed1d..445063d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1389,7 +1389,7 @@ libvirt_driver_hyperv_la_LIBADD = $(OPENWSMAN_LIBS)
 libvirt_driver_

[libvirt] [PATCH 2/4] parallels: substitute parallels with vz in strings

[Maxim Nestratov]

Here we stop referencing vz driver by different names
in error messages. 'parallels driver', 'Parallels Cloud
Server', 'Parallels driver' all become just 'vz driver'.
No functional changes. Only renaming and a bit of rewording.

Signed-off-by: Maxim Nestratov 
---
 src/parallels/parallels_sdk.c | 162 +-
 1 file changed, 81 insertions(+), 81 deletions(-)

diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
index 53a4376..4dafe7c 100644
--- a/src/parallels/parallels_sdk.c
+++ b/src/parallels/parallels_sdk.c
@@ -1912,21 +1912,21 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, 
virDomainDefPtr def)
 
 if (def->title) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("titles are not supported by parallels driver"));
+   _("titles are not supported by vz driver"));
 return -1;
 }
 
 if (def->blkio.ndevices > 0) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("blkio parameters are not supported "
- "by parallels driver"));
+ "by vz driver"));
 return -1;
 }
 
 if (virDomainDefGetMemoryActual(def) != def->mem.cur_balloon) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("changing balloon parameters is not supported "
- "by parallels driver"));
+ "by vz driver"));
return -1;
 }
 
@@ -1944,7 +1944,7 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, 
virDomainDefPtr def)
 
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Memory parameter is not supported "
- "by parallels driver"));
+ "by vz driver"));
 return -1;
 }
 
@@ -1957,7 +1957,7 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, 
virDomainDefPtr def)
 if (def->placement_mode) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("changing cpu placement mode is not supported "
- "by parallels driver"));
+ "by vz driver"));
 return -1;
 }
 
@@ -1967,7 +1967,7 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, 
virDomainDefPtr def)
 def->cputune.quota) {
 
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("cputune is not supported by parallels driver"));
+   _("cputune is not supported by vz driver"));
 return -1;
 }
 
@@ -1993,7 +1993,7 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, 
virDomainDefPtr def)
  virDomainNumatuneHasPerNodeBinding(def->numa)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
 _("numa parameters are not supported "
-  "by parallels driver"));
+  "by vz driver"));
 return -1;
 }
 
@@ -2003,7 +2003,7 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, 
virDomainDefPtr def)
 
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("on_reboot, on_poweroff and on_crash parameters "
- "are not supported by parallels driver"));
+ "are not supported by vz driver"));
 return -1;
 }
 
@@ -2020,7 +2020,7 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, 
virDomainDefPtr def)
 
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("changing OS parameters is not supported "
- "by parallels driver"));
+ "by vz driver"));
 return -1;
 }
 
@@ -2035,7 +2035,7 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, 
virDomainDefPtr def)
 
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("changing OS type is not supported "
- "by parallels driver"));
+ "by vz driver"));
 return -1;
 }
 
@@ -2046,7 +2046,7 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, 
virDomainDefPtr def)
 
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("changing OS parameters is not supported "
- "by parallels driver"));
+ "by vz driver"));
 return -1;
 }
 } else {
@@ -2056,7 +2056,7 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, 
virDomainDefPtr def)
 
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("changing OS parameters is not supported "
- "by parallels driver"));
+ "by vz driver"));
 return -1;
 }
 }
@@ -2064,7 +2064,7 @@ prlsdkCheckUnsupportedParams(PRL_HANDLE sdkdom, 
virDomainDefPtr def)
 if (def->emulator) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
  

[libvirt] [PATCH 1/4] parallels: change parallels prefixes to vz

[Maxim Nestratov]

This patch changes all parallels/vz driver structure and
function prefixes from parallels to vz.
No functional changes.

Signed-off-by: Maxim Nestratov 
---
 src/libvirt.c |   2 +-
 src/parallels/parallels_driver.c  | 378 -
 src/parallels/parallels_driver.h  |   2 +-
 src/parallels/parallels_network.c | 138 ++--
 src/parallels/parallels_sdk.c |  78 +++
 src/parallels/parallels_sdk.h |  18 +-
 src/parallels/parallels_storage.c | 436 +++---
 src/parallels/parallels_utils.c   |  28 +--
 src/parallels/parallels_utils.h   |  52 ++---
 9 files changed, 566 insertions(+), 566 deletions(-)

diff --git a/src/libvirt.c b/src/libvirt.c
index fe62faa..f1e8d3f 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -434,7 +434,7 @@ virGlobalInit(void)
 goto error;
 # endif
 # ifdef WITH_PARALLELS
-if (parallelsRegister() == -1)
+if (vzRegister() == -1)
 goto error;
 # endif
 #endif
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 706229d..5a18b06 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -64,22 +64,22 @@ VIR_LOG_INIT("parallels.parallels_driver");
 #define PRLCTL  "prlctl"
 #define PRLSRVCTL   "prlsrvctl"
 
-static int parallelsConnectClose(virConnectPtr conn);
+static int vzConnectClose(virConnectPtr conn);
 
 void
-parallelsDriverLock(parallelsConnPtr driver)
+vzDriverLock(vzConnPtr driver)
 {
 virMutexLock(&driver->lock);
 }
 
 void
-parallelsDriverUnlock(parallelsConnPtr driver)
+vzDriverUnlock(vzConnPtr driver)
 {
 virMutexUnlock(&driver->lock);
 }
 
 static virCapsPtr
-parallelsBuildCapabilities(void)
+vzBuildCapabilities(void)
 {
 virCapsPtr caps = NULL;
 virCPUDefPtr cpu = NULL;
@@ -177,19 +177,19 @@ parallelsBuildCapabilities(void)
 }
 
 static char *
-parallelsConnectGetCapabilities(virConnectPtr conn)
+vzConnectGetCapabilities(virConnectPtr conn)
 {
-parallelsConnPtr privconn = conn->privateData;
+vzConnPtr privconn = conn->privateData;
 char *xml;
 
-parallelsDriverLock(privconn);
+vzDriverLock(privconn);
 xml = virCapabilitiesFormatXML(privconn->caps);
-parallelsDriverUnlock(privconn);
+vzDriverUnlock(privconn);
 return xml;
 }
 
 static int
-parallelsDomainDefPostParse(virDomainDefPtr def,
+vzDomainDefPostParse(virDomainDefPtr def,
 virCapsPtr caps ATTRIBUTE_UNUSED,
 void *opaque ATTRIBUTE_UNUSED)
 {
@@ -201,7 +201,7 @@ parallelsDomainDefPostParse(virDomainDefPtr def,
 }
 
 static int
-parallelsDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
+vzDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
   const virDomainDef *def,
   virCapsPtr caps ATTRIBUTE_UNUSED,
   void *opaque ATTRIBUTE_UNUSED)
@@ -222,17 +222,17 @@ parallelsDomainDeviceDefPostParse(virDomainDeviceDefPtr 
dev,
 }
 
 
-virDomainDefParserConfig parallelsDomainDefParserConfig = {
+virDomainDefParserConfig vzDomainDefParserConfig = {
 .macPrefix = {0x42, 0x1C, 0x00},
-.devicesPostParseCallback = parallelsDomainDeviceDefPostParse,
-.domainPostParseCallback = parallelsDomainDefPostParse,
+.devicesPostParseCallback = vzDomainDeviceDefPostParse,
+.domainPostParseCallback = vzDomainDefPostParse,
 };
 
 
 static int
-parallelsOpenDefault(virConnectPtr conn)
+vzOpenDefault(virConnectPtr conn)
 {
-parallelsConnPtr privconn;
+vzConnPtr privconn;
 
 if (VIR_ALLOC(privconn) < 0)
 return VIR_DRV_OPEN_ERROR;
@@ -252,10 +252,10 @@ parallelsOpenDefault(virConnectPtr conn)
 if (prlsdkConnect(privconn) < 0)
 goto err_free;
 
-if (!(privconn->caps = parallelsBuildCapabilities()))
+if (!(privconn->caps = vzBuildCapabilities()))
 goto error;
 
-if (!(privconn->xmlopt = 
virDomainXMLOptionNew(¶llelsDomainDefParserConfig,
+if (!(privconn->xmlopt = virDomainXMLOptionNew(&vzDomainDefParserConfig,
  NULL, NULL)))
 goto error;
 
@@ -288,7 +288,7 @@ parallelsOpenDefault(virConnectPtr conn)
 }
 
 static virDrvOpenStatus
-parallelsConnectOpen(virConnectPtr conn,
+vzConnectOpen(virConnectPtr conn,
  virConnectAuthPtr auth ATTRIBUTE_UNUSED,
  unsigned int flags)
 {
@@ -324,10 +324,10 @@ parallelsConnectOpen(virConnectPtr conn,
 return VIR_DRV_OPEN_ERROR;
 }
 
-if ((ret = parallelsOpenDefault(conn)) != VIR_DRV_OPEN_SUCCESS ||
-(ret = parallelsStorageOpen(conn, flags)) != VIR_DRV_OPEN_SUCCESS ||
-(ret = parallelsNetworkOpen(conn, flags)) != VIR_DRV_OPEN_SUCCESS) {
-parallelsConnectClose(conn);
+if ((ret = vzOpenDefault(conn)) != VIR_DRV_OPEN_SUCCESS ||
+(ret = vzStorageOpen(conn, flags)) != VIR_DRV_OPEN_SUCCESS ||
+

[libvirt] [PATCH 3/4] parallels: rename all parallels files and driver directory to vz

[Maxim Nestratov]

This patch moves all src/parallels/parallels* files to vz/vz*
and fixes build accordingly.
No functional changes.

Signed-off-by: Maxim Nestratov 
---
 po/POTFILES.in | 12 
 src/Makefile.am| 32 +++---
 src/libvirt.c  |  2 +-
 .../parallels_driver.c => vz/vz_driver.c}  |  8 +++---
 .../parallels_driver.h => vz/vz_driver.h}  |  2 +-
 .../parallels_network.c => vz/vz_network.c}|  4 +--
 src/{parallels/parallels_sdk.c => vz/vz_sdk.c} |  4 +--
 src/{parallels/parallels_sdk.h => vz/vz_sdk.h} |  4 +--
 .../parallels_storage.c => vz/vz_storage.c}|  4 +--
 src/{parallels/parallels_utils.c => vz/vz_utils.c} |  4 +--
 src/{parallels/parallels_utils.h => vz/vz_utils.h} |  2 +-
 11 files changed, 39 insertions(+), 39 deletions(-)
 rename src/{parallels/parallels_driver.c => vz/vz_driver.c} (99%)
 rename src/{parallels/parallels_driver.h => vz/vz_driver.h} (93%)
 rename src/{parallels/parallels_network.c => vz/vz_network.c} (99%)
 rename src/{parallels/parallels_sdk.c => vz/vz_sdk.c} (99%)
 rename src/{parallels/parallels_sdk.h => vz/vz_sdk.h} (96%)
 rename src/{parallels/parallels_storage.c => vz/vz_storage.c} (99%)
 rename src/{parallels/parallels_utils.c => vz/vz_utils.c} (98%)
 rename src/{parallels/parallels_utils.h => vz/vz_utils.h} (98%)

diff --git a/po/POTFILES.in b/po/POTFILES.in
index bb0f6e1..ae41b34 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -104,12 +104,12 @@ src/nwfilter/nwfilter_learnipaddr.c
 src/openvz/openvz_conf.c
 src/openvz/openvz_driver.c
 src/openvz/openvz_util.c
-src/parallels/parallels_driver.c
-src/parallels/parallels_network.c
-src/parallels/parallels_sdk.c
-src/parallels/parallels_utils.c
-src/parallels/parallels_utils.h
-src/parallels/parallels_storage.c
+src/vz/vz_driver.c
+src/vz/vz_network.c
+src/vz/vz_sdk.c
+src/vz/vz_utils.c
+src/vz/vz_utils.h
+src/vz/vz_storage.c
 src/phyp/phyp_driver.c
 src/qemu/qemu_agent.c
 src/qemu/qemu_blockjob.c
diff --git a/src/Makefile.am b/src/Makefile.am
index 0d1f58b..dc9ed1d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -534,7 +534,7 @@ DRIVER_SOURCE_FILES = \
$(NODE_DEVICE_DRIVER_UDEV_SOURCES) \
$(NWFILTER_DRIVER_SOURCES) \
$(OPENVZ_DRIVER_SOURCES) \
-   $(PARALLELS_DRIVER_SOURCES) \
+   $(VZ_DRIVER_SOURCES) \
$(PHYP_DRIVER_SOURCES) \
$(QEMU_DRIVER_SOURCES) \
$(REMOTE_DRIVER_SOURCES) \
@@ -802,15 +802,15 @@ HYPERV_DRIVER_EXTRA_DIST =
\
hyperv/hyperv_wmi_generator.py  
\
$(HYPERV_DRIVER_GENERATED)
 
-PARALLELS_DRIVER_SOURCES = \
-   parallels/parallels_driver.h\
-   parallels/parallels_driver.c\
-   parallels/parallels_utils.c \
-   parallels/parallels_utils.h \
-   parallels/parallels_storage.c   \
-   parallels/parallels_network.c   \
-   parallels/parallels_sdk.h   \
-   parallels/parallels_sdk.c
+VZ_DRIVER_SOURCES =\
+   vz/vz_driver.h  \
+   vz/vz_driver.c  \
+   vz/vz_utils.c   \
+   vz/vz_utils.h   \
+   vz/vz_storage.c \
+   vz/vz_network.c \
+   vz/vz_sdk.h \
+   vz/vz_sdk.c
 
 BHYVE_DRIVER_SOURCES = \
bhyve/bhyve_capabilities.c  \
@@ -1390,13 +1390,13 @@ libvirt_driver_hyperv_la_SOURCES = 
$(HYPERV_DRIVER_SOURCES)
 endif WITH_HYPERV
 
 if WITH_PARALLELS
-noinst_LTLIBRARIES += libvirt_driver_parallels.la
-libvirt_la_BUILT_LIBADD += libvirt_driver_parallels.la
-libvirt_driver_parallels_la_CFLAGS = \
+noinst_LTLIBRARIES += libvirt_driver_vz.la
+libvirt_la_BUILT_LIBADD += libvirt_driver_vz.la
+libvirt_driver_vz_la_CFLAGS = \
-I$(srcdir)/conf $(AM_CFLAGS) \
$(PARALLELS_SDK_CFLAGS) $(LIBNL_CFLAGS)
-libvirt_driver_parallels_la_LIBADD = $(PARALLELS_SDK_LIBS) $(LIBNL_LIBS)
-libvirt_driver_parallels_la_SOURCES = $(PARALLELS_DRIVER_SOURCES)
+libvirt_driver_vz_la_LIBADD = $(PARALLELS_SDK_LIBS) $(LIBNL_LIBS)
+libvirt_driver_vz_la_SOURCES = $(VZ_DRIVER_SOURCES)
 endif WITH_PARALLELS
 
 if WITH_BHYVE
@@ -1762,7 +1762,7 @@ EXTRA_DIST += 
\
$(ESX_DRIVER_EXTRA_DIST)\
$(HYPERV_DRIVER_SOURCES)\
$(HYPERV_DRIVER_EXTRA_DIST) \
-   $(PARALLELS_DRIVER_SOURCES)

[libvirt] [PATCH 0/4] parallels: rename parallels to vz

[Maxim Nestratov]

This change is made in the scope of 'Parallel Cloud Server'
pruduct renaming to 'Virtuozzo' to prevent future confusion and make
things actual.
Previously we introduced a new driver name 'vz' as a synonym to 'parallels'.
This patch set finalizes major part of renaming process.
Though uri 'parallels:///system' and 'parallels' domain type
remains valid, we are going to reference the driver and all
changes related to it as 'vz' since now.

Maxim Nestratov (4):
  parallels: change parallels prefixes to vz
  parallels: substitute parallels with vz in strings
  parallels: rename all parallels files and driver directory to vz
  parallels: substitute parallels with vz spec file and Makefile

 configure.ac   |  26 +-
 libvirt.spec.in|  10 +-
 mingw-libvirt.spec.in  |   6 +-
 po/POTFILES.in |  12 +-
 src/Makefile.am|  36 +-
 src/libvirt.c  |  10 +-
 .../parallels_driver.c => vz/vz_driver.c}  | 386 +-
 .../parallels_driver.h => vz/vz_driver.h}  |   4 +-
 .../parallels_network.c => vz/vz_network.c}| 142 +++
 src/{parallels/parallels_sdk.c => vz/vz_sdk.c} | 244 ++--
 src/{parallels/parallels_sdk.h => vz/vz_sdk.h} |  22 +-
 .../parallels_storage.c => vz/vz_storage.c}| 440 ++---
 src/{parallels/parallels_utils.c => vz/vz_utils.c} |  32 +-
 src/{parallels/parallels_utils.h => vz/vz_utils.h} |  54 +--
 14 files changed, 712 insertions(+), 712 deletions(-)
 rename src/{parallels/parallels_driver.c => vz/vz_driver.c} (73%)
 rename src/{parallels/parallels_driver.h => vz/vz_driver.h} (90%)
 rename src/{parallels/parallels_network.c => vz/vz_network.c} (75%)
 rename src/{parallels/parallels_sdk.c => vz/vz_sdk.c} (93%)
 rename src/{parallels/parallels_sdk.h => vz/vz_sdk.h} (75%)
 rename src/{parallels/parallels_storage.c => vz/vz_storage.c} (74%)
 rename src/{parallels/parallels_utils.c => vz/vz_utils.c} (84%)
 rename src/{parallels/parallels_utils.h => vz/vz_utils.h} (68%)

-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 11/22] qemu_monitor: Wire up SPICE_MIGRATE_COMPLETED event

[Peter Krempa]

On Tue, Jun 02, 2015 at 14:34:16 +0200, Jiri Denemark wrote:
> Signed-off-by: Jiri Denemark 
> ---
>

ACK,

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 v2 12/22] qemu: Do not poll for spice migration status

[Peter Krempa]

On Tue, Jun 02, 2015 at 14:34:17 +0200, Jiri Denemark wrote:
> QEMU_CAPS_SEAMLESS_MIGRATION capability says QEMU supports
> SPICE_MIGRATE_COMPLETED event. Thus we can just drop all code which
> polls query-spice and replace it with waiting for the event.
> 
> Signed-off-by: Jiri Denemark 
> ---

ACK,
Peter


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] add some more missing libvirt functions

[Vasiliy Tolstov]

* libvirt_connect_get_all_domain_stats
* libvirt_domain_block_resize
* libvirt_domain_block_job_abort
* libvirt_domain_block_job_set_speed

Signed-off-by: Vasiliy Tolstov 
---
 src/libvirt-php.c | 177 +-
 src/libvirt-php.h |   4 ++
 2 files changed, 180 insertions(+), 1 deletion(-)

diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index e9b9657..f9096ef 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -91,6 +91,7 @@ static zend_function_entry libvirt_functions[] = {
PHP_FE(libvirt_connect_get_maxvcpus, NULL)
PHP_FE(libvirt_connect_get_encrypted, NULL)
PHP_FE(libvirt_connect_get_secure, NULL)
+   PHP_FE(libvirt_connect_get_all_domain_stats, NULL)
/* Stream functions */
PHP_FE(libvirt_stream_create, NULL)
PHP_FE(libvirt_stream_close, NULL)
@@ -136,6 +137,10 @@ static zend_function_entry libvirt_functions[] = {
PHP_FE(libvirt_domain_memory_peek,NULL)
PHP_FE(libvirt_domain_memory_stats,NULL)
PHP_FE(libvirt_domain_block_stats,NULL)
+   PHP_FE(libvirt_domain_block_resize,NULL)
+   //  PHP_FE(libvirt_domain_block_copy,NULL)
+   PHP_FE(libvirt_domain_block_job_abort,NULL)
+   PHP_FE(libvirt_domain_block_job_set_speed,NULL)
PHP_FE(libvirt_domain_interface_stats,NULL)
PHP_FE(libvirt_domain_get_connect, NULL)
PHP_FE(libvirt_domain_migrate, NULL)
@@ -1332,6 +1337,11 @@ PHP_MINIT_FUNCTION(libvirt)
/* Job was aborted but it's not cleanup up yet */
REGISTER_LONG_CONSTANT("VIR_DOMAIN_JOB_CANCELLED",  5, CONST_CS | 
CONST_PERSISTENT);
 
+   REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC",  
VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC, CONST_CS | CONST_PERSISTENT);
+   REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT",  
VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT, CONST_CS | CONST_PERSISTENT);
+
+   REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_JOB_SPEED_BANDWIDTH_BYTES",
VIR_DOMAIN_BLOCK_JOB_SPEED_BANDWIDTH_BYTES, CONST_CS | CONST_PERSISTENT);
+
/* Migration constants */
REGISTER_LONG_CONSTANT("VIR_MIGRATE_LIVE",1, CONST_CS | 
CONST_PERSISTENT);
/* direct source -> dest host control channel Note the less-common 
spelling that we're stuck with: */
@@ -1374,7 +1384,7 @@ PHP_MINIT_FUNCTION(libvirt)
REGISTER_LONG_CONSTANT("VIR_DOMAIN_FLAG_TEST_LOCAL_VNC",
DOMAIN_FLAG_TEST_LOCAL_VNC, CONST_CS | CONST_PERSISTENT);
REGISTER_LONG_CONSTANT("VIR_DOMAIN_FLAG_SOUND_AC97",
DOMAIN_FLAG_SOUND_AC97, CONST_CS | CONST_PERSISTENT);
REGISTER_LONG_CONSTANT("VIR_DOMAIN_DISK_FILE",  
DOMAIN_DISK_FILE, CONST_CS | CONST_PERSISTENT);
-   REGISTER_LONG_CONSTANT("VIR_DOMAIN_DISK_BLOCK", 
DOMAIN_DISK_BLOCK, CONST_CS | CONST_PERSISTENT);
+   REGISTER_LONG_CONSTANT("VIR_DOMAIN_DISK_BLOCK", 
DOMAIN_DISK_BLOCK, CONST_CS | CONST_PERSISTENT);
REGISTER_LONG_CONSTANT("VIR_DOMAIN_DISK_ACCESS_ALL",
DOMAIN_DISK_ACCESS_ALL, CONST_CS | CONST_PERSISTENT);
 
/* Domain metadata constants */
@@ -1385,6 +1395,24 @@ PHP_MINIT_FUNCTION(libvirt)
REGISTER_LONG_CONSTANT("VIR_DOMAIN_AFFECT_LIVE",1, 
CONST_CS | CONST_PERSISTENT);
REGISTER_LONG_CONSTANT("VIR_DOMAIN_AFFECT_CONFIG",  2, 
CONST_CS | CONST_PERSISTENT);
 
+   REGISTER_LONG_CONSTANT("VIR_DOMAIN_STATS_STATE",
VIR_DOMAIN_STATS_STATE, CONST_CS | CONST_PERSISTENT);
+   REGISTER_LONG_CONSTANT("VIR_DOMAIN_STATS_CPU_TOTAL",
VIR_DOMAIN_STATS_CPU_TOTAL, CONST_CS | CONST_PERSISTENT);
+   REGISTER_LONG_CONSTANT("VIR_DOMAIN_STATS_BALLOON",  
VIR_DOMAIN_STATS_BALLOON, CONST_CS | CONST_PERSISTENT);
+   REGISTER_LONG_CONSTANT("VIR_DOMAIN_STATS_VCPU", 
VIR_DOMAIN_STATS_VCPU, CONST_CS | CONST_PERSISTENT);
+   REGISTER_LONG_CONSTANT("VIR_DOMAIN_STATS_INTERFACE",
VIR_DOMAIN_STATS_INTERFACE, CONST_CS | CONST_PERSISTENT);
+   REGISTER_LONG_CONSTANT("VIR_DOMAIN_STATS_BLOCK",
VIR_DOMAIN_STATS_BLOCK, CONST_CS | CONST_PERSISTENT);
+
+   REGISTER_LONG_CONSTANT("VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE",  
VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE, CONST_CS | CONST_PERSISTENT);
+   REGISTER_LONG_CONSTANT("VIR_CONNECT_GET_ALL_DOMAINS_STATS_INACTIVE",
VIR_CONNECT_GET_ALL_DOMAINS_STATS_INACTIVE, CONST_CS | CONST_PERSISTENT);
+   REGISTER_LONG_CONSTANT("VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER",   
VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER, CONST_CS | CONST_PERSISTENT);
+   REGISTER_LONG_CONSTANT("VIR_CONNECT_GET_ALL_DOMAINS_STATS_PAUSED",  
VIR_CONNECT_GET_ALL_DOMAINS_STATS_PAUSED, CONST_CS | CONST_PERSISTENT);
+   REGISTER_LONG_CONSTANT("VIR_CONNECT_GET_ALL_DOMAINS_STATS_PERSISTENT",  
VIR_CONNECT_GET_ALL_DOMAINS_STATS_PERSISTENT, CONST_CS | CONST_PERSISTENT);
+   REGISTER_LONG_CONSTANT("VIR_CONN

Re: [libvirt] [PATCH 0/8] logically memory hotplug via guest agent

[Vasiliy Tolstov]

2015-06-10 11:37 GMT+03:00 Daniel P. Berrange :
> The udev rules are really something the OS vendor should setup, so
> that it "just works"


I think so, also for vcpu hotplug this also covered by udev. May be we
need something to hot remove memory and cpu, because in guest we need
offline firstly.

-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 08/22] qemu: Refactor qemuMonitorBlockJobInfo

[Peter Krempa]

On Tue, Jun 02, 2015 at 14:34:13 +0200, Jiri Denemark wrote:
> "query-block-jobs" QMP command returns all running block jobs at once,
> while qemuMonitorBlockJobInfo would only report one. This is not very
> nice in case we need to check several block jobs. This patch refactors
> the monitor code to always parse all block jobs and store them in a
> hash.
> 
> Signed-off-by: Jiri Denemark 
> ---
> 
> Notes:
> Version 2:
> - new patch
>

ACK,

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 0/8] logically memory hotplug via guest agent

[Daniel P. Berrange]

On Wed, Jun 10, 2015 at 02:05:16PM +0800, zhang bo wrote:
> On 2015/6/10 13:40, Vasiliy Tolstov wrote:
> 
> > 2015-06-10 5:28 GMT+03:00 zhang bo :
> >> Thank you for your reply.
> >> Before this patch, we needed to manually online memory blocks inside the 
> >> guest, after dimm memory hotplug
> >> for most *nix OSes. (Windows guests automatically get their memory blocks 
> >> online after hotplugging)
> >> That is to say, we need to LOGICALLY hotplug memory after PHYSICAL hotplug.
> >> This patch did the LOGICAL part.
> >> With this patch, we don't need to get into the guest to manually online 
> >> them anymore, which is even
> >> impossible for most host administrators.
> > 
> > 
> > As i remember this online step easy can be automate via udev rules.
> > 
> 
> 
> Logically that's true, but adding udev rules means:
> 1 you have to get into the guest
> 2 you have to be familar with udev rules.
> 
> Not convenient enough compared to just calling libvirt API to do so.

The udev rules are really something the OS vendor should setup, so
that it "just works"

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 v2 09/22] qemu: Cancel disk mirrors after libvirtd restart

[Peter Krempa]

On Tue, Jun 02, 2015 at 14:34:14 +0200, Jiri Denemark wrote:
> When libvirtd is restarted during migration, we properly cancel the
> ongoing migration (unless it managed to almost finished before the
> restart). But if we were also migrating storage using NBD, we would
> completely forget about the running disk mirrors.
> 
> Signed-off-by: Jiri Denemark 
> ---
>

ACK,

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 v2 10/22] qemu: Use domain condition for asyncAbort

[Peter Krempa]

On Tue, Jun 02, 2015 at 14:34:15 +0200, Jiri Denemark wrote:
> To avoid polling for asyncAbort flag changes.
> 
> Signed-off-by: Jiri Denemark 
> ---

ACK,

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 v2 15/22] qemu: Don't pass redundant job name around

[Peter Krempa]

On Tue, Jun 02, 2015 at 14:34:20 +0200, Jiri Denemark wrote:
> Instead of passing current job name to several functions which already
> know what the current job is we can generate the name where we actually
> need to use it.
> 
> Signed-off-by: Jiri Denemark 
> ---
>

ACK,

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] virNumaSetPagePoolSize: Produce friendlier error message

[Michal Privoznik]

On 09.06.2015 08:42, Martin Kletzander wrote:
> On Mon, Jun 08, 2015 at 02:41:05PM +0200, Michal Privoznik wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1224587
>>
>> The function takes two important arguments (among many others): @node
>> and @page_size. From these two a path under /sys is constructed. The
>> path is then used to read and write the desired size of huge pages
>> pool. However, if the path does not exists due to either @node or
>> @page_size having nonexistent value (e.g. there's no such NUMA node or
>> no page size like -2), an cryptic error message is produced:
>>
>>  virsh # allocpages --pagesize 2049 --pagecount 8 --cellno -2
>>  error: Failed to open file
>> '/sys/devices/system/node/node-2/hugepages/hugepages-2049kB/nr_hugepages':
>> No such file or directory
>>
>> Add two more checks to catch this and therefore produce much more
>> friendlier error messages.
>>
>> Signed-off-by: Michal Privoznik 
>> ---
>> src/util/virnuma.c | 18 ++
>> 1 file changed, 18 insertions(+)
>>
>> diff --git a/src/util/virnuma.c b/src/util/virnuma.c
>> index 669192a..5807d8f 100644
>> --- a/src/util/virnuma.c
>> +++ b/src/util/virnuma.c
>> @@ -849,9 +849,27 @@ virNumaSetPagePoolSize(int node,
>> goto cleanup;
>> }
>>
>> +if (node != -1 && !virNumaNodeIsAvailable(node)) {
>> +virReportError(VIR_ERR_OPERATION_FAILED,
>> +   _("NUMA node %d is not available"),
>> +   node);
>> +goto cleanup;
>> +}
>> +
>> if (virNumaGetHugePageInfoPath(&nr_path, node, page_size,
>> "nr_hugepages") < 0)
>> goto cleanup;
>>
>> +if (!virFileExists(nr_path)) {
>> +/* Strictly speaking, @nr_path contains both NUMA node and
>> page size.
>> + * So if it doesn't exist it can be due to any of those two
>> is wrong.
>> + * However, the existence of the node was checked a few lines
>> above, so
>> + * it can be only page size here. */
> 
> Über-strictly speaking, unless you compile with both WITH_NUMACTL &&
> HAVE_NUMA_BITMASK_ISBITSET then virNumaNodeIsAvailable() can pass for
> invalid node in case of non-contiguous NUMA node numbers.

So what are you saying is that I should update the comment or the error
message or leave everything as-is and push it?

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 13/22] qemu: Refactor qemuDomainGetJob{Info, Stats}

[Peter Krempa]

On Tue, Jun 02, 2015 at 14:34:18 +0200, Jiri Denemark wrote:
> Move common parts of qemuDomainGetJobInfo and qemuDomainGetJobStats into
> a separate API (qemuDomainGetJobStatsInternal).
> 
> Signed-off-by: Jiri Denemark 
> ---
> 

ACK,

Peter


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [libvirt-php][PATCH 00/10] Couple of PHP fixes

[Michal Privoznik]

On 05.05.2015 11:59, Michal Privoznik wrote:
> Even though this project is not actively developed, it does not
> mean we can't make some things better in it. After this patchset,
> distcheck works, targets are not built everytime, only when
> necessary, and so on.
> 
> Michal Privoznik (10):
>   Update .gitignore
>   Update autotools generated files
>   get_next_free_numeric_value: Use correct format for sscanf()
>   tools: Cleanup Makefile
>   configure.ac: Check for libtool
>   src: Clean up Makefile
>   tests: Clean up Makefile.am
>   tests: Update qemu path
>   tests: run under distcheck
>   Install libvirt-php.ini more wisely
> 
>  .gitignore   |  28 ++
>  INSTALL  |   8 +-
>  Makefile.am  |   4 +-
>  configure.ac |   8 +-
>  install-sh   |  14 +-
>  missing  | 412 
> ++-
>  src/Makefile.am  |  64 +++--
>  src/libvirt-php.c|   2 +-
>  tests/Makefile.am|  37 ++-
>  tests/data/example-qcow2-disk.xml|   2 +-
>  tests/{functions.phpt => functions.phpt.in}  |   4 +-
>  tests/php.ini|   2 +-
>  tests/runtests.sh|   4 +-
>  tests/test-domain-create-and-coredump.phpt   |   3 +-
>  tests/test-domain-create-and-get-xpath.phpt  |   3 +-
>  tests/test-domain-create-get-metadata.phpt   |   3 +-
>  tests/test-domain-create.phpt|   3 +-
>  tests/test-domain-define-create-destroy.phpt |   3 +-
>  tests/test-domain-define-undefine.phpt   |   3 +-
>  tests/test-domain-snapshot.phpt  |   3 +-
>  tools/Makefile.am|  29 +-
>  21 files changed, 309 insertions(+), 330 deletions(-)
>  rename tests/{functions.phpt => functions.phpt.in} (91%)
> 

Ping? No php knowledge is required to review these patches. They are
merely Makefile and configure.ac cleanups.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/8] logically memory hotplug via guest agent

[zhang bo]

On 2015/6/10 16:39, Vasiliy Tolstov wrote:

> 2015-06-10 11:37 GMT+03:00 Daniel P. Berrange :
>> The udev rules are really something the OS vendor should setup, so
>> that it "just works"
> 
> 
> I think so, also for vcpu hotplug this also covered by udev. May be we
> need something to hot remove memory and cpu, because in guest we need
> offline firstly.
> 


In fact ,we also have --guest option for 'virsh sevvcpus' command, which also
uses qga commands to do the logical hotplug/unplug jobs, although udev rules 
seems
to cover the vcpu logical hotplug issue.

virsh # help setvcpus
.
--guest  modify cpu state in the guest


BTW: we didn't see OSes with udev rules for memory-hotplug-event setted by 
vendors, 
and adding such rules means that we have to *interfere within the guest*, It 
seems 
not a good option.

-- 
Oscar
oscar.zhan...@huawei.com  

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 0/8] logically memory hotplug via guest agent

[Daniel P. Berrange]

On Wed, Jun 10, 2015 at 05:24:50PM +0800, zhang bo wrote:
> On 2015/6/10 16:39, Vasiliy Tolstov wrote:
> 
> > 2015-06-10 11:37 GMT+03:00 Daniel P. Berrange :
> >> The udev rules are really something the OS vendor should setup, so
> >> that it "just works"
> > 
> > 
> > I think so, also for vcpu hotplug this also covered by udev. May be we
> > need something to hot remove memory and cpu, because in guest we need
> > offline firstly.
> > 
> 
> 
> In fact ,we also have --guest option for 'virsh sevvcpus' command, which also
> uses qga commands to do the logical hotplug/unplug jobs, although udev rules 
> seems
> to cover the vcpu logical hotplug issue.
> 
> virsh # help setvcpus
> .
> --guest  modify cpu state in the guest
> 
> 
> BTW: we didn't see OSes with udev rules for memory-hotplug-event setted by 
> vendors, 
> and adding such rules means that we have to *interfere within the guest*, It 
> seems 
> not a good option.

I was suggesting that an RFE be filed with any vendor who doesn't do it
to add this capability, not that we add udev rules ourselves.

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 0/8] logically memory hotplug via guest agent

[Daniel P. Berrange]

On Wed, Jun 10, 2015 at 10:28:08AM +0100, Daniel P. Berrange wrote:
> On Wed, Jun 10, 2015 at 05:24:50PM +0800, zhang bo wrote:
> > On 2015/6/10 16:39, Vasiliy Tolstov wrote:
> > 
> > > 2015-06-10 11:37 GMT+03:00 Daniel P. Berrange :
> > >> The udev rules are really something the OS vendor should setup, so
> > >> that it "just works"
> > > 
> > > 
> > > I think so, also for vcpu hotplug this also covered by udev. May be we
> > > need something to hot remove memory and cpu, because in guest we need
> > > offline firstly.
> > > 
> > 
> > 
> > In fact ,we also have --guest option for 'virsh sevvcpus' command, which 
> > also
> > uses qga commands to do the logical hotplug/unplug jobs, although udev 
> > rules seems
> > to cover the vcpu logical hotplug issue.
> > 
> > virsh # help setvcpus
> > .
> > --guest  modify cpu state in the guest
> > 
> > 
> > BTW: we didn't see OSes with udev rules for memory-hotplug-event setted by 
> > vendors, 
> > and adding such rules means that we have to *interfere within the guest*, 
> > It seems 
> > not a good option.
> 
> I was suggesting that an RFE be filed with any vendor who doesn't do it
> to add this capability, not that we add udev rules ourselves.

Or actually, it probably is sufficient to just send a patch to the upstream
systemd project to add the desired rule to udev. That way all Linux distros
will inherit the feature when they update to new udev.

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] [libvirt-php][PATCH 00/10] Couple of PHP fixes

[Vasiliy Tolstov]

2015-06-10 12:23 GMT+03:00 Michal Privoznik :
> Ping? No php knowledge is required to review these patches. They are
> merely Makefile and configure.ac cleanups.
>
> Michal


Michal Novotny says that he busy now and you can accept/review my
patches.. i think in this case - you can ACK ? =)

-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 14/22] qemu: Refactor qemuMigrationUpdateJobStatus

[Peter Krempa]

On Tue, Jun 02, 2015 at 14:34:19 +0200, Jiri Denemark wrote:
> Once we start waiting for migration events instead of polling
> query-migrate, priv->job.current will not be regularly updated anymore
> because we will get the current status directly from the events. Thus
> virDomainGetJob{Info,Stats} will have to query QEMU, but they can't just
> blindly update priv->job.current structure. This patch introduces
> qemuMigrationFetchJobStatus which just fills in a caller supplied
> structure and makes qemuMigrationUpdateJobStatus a tiny wrapper around
> it.
> 
> Signed-off-by: Jiri Denemark 
> ---
>

ACK,

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 v1] update snapshot api

[Michal Privoznik]

On 07.05.2015 16:21, Vasiliy Tolstov wrote:
> * add constants from libvirt to snapshots api
> * add flags to snapshot functions
> 
> Signed-off-by: Vasiliy Tolstov 
> ---
>  src/libvirt-php.c | 87 
> ++-
>  1 file changed, 61 insertions(+), 26 deletions(-)
> 

There should be [libvirt-php] in the subject. You can achieve that
(permanently) via:

  libvirt-php.git $ git config format.subjectprefix libvirt-php][PATCH


> diff --git a/src/libvirt-php.c b/src/libvirt-php.c
> index 0adc4be..e9b9657 100644
> --- a/src/libvirt-php.c
> +++ b/src/libvirt-php.c
> @@ -1228,34 +1228,58 @@ PHP_MINIT_FUNCTION(libvirt)
>   REGISTER_LONG_CONSTANT("VIR_DOMAIN_CRASHED",6, CONST_CS | 
> CONST_PERSISTENT);
>  
>  /* Volume constants */
> -REGISTER_LONG_CONSTANT("VIR_STORAGE_VOL_RESIZE_ALLOCATE",1, 
> CONST_CS | CONST_PERSISTENT);
> -REGISTER_LONG_CONSTANT("VIR_STORAGE_VOL_RESIZE_DELTA",   2, 
> CONST_CS | CONST_PERSISTENT);
> -REGISTER_LONG_CONSTANT("VIR_STORAGE_VOL_RESIZE_SHRINK",  4, 
> CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_STORAGE_VOL_RESIZE_ALLOCATE",1, 
> CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_STORAGE_VOL_RESIZE_DELTA",   2, 
> CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_STORAGE_VOL_RESIZE_SHRINK",  4, 
> CONST_CS | CONST_PERSISTENT);

This change looks questionable to me. Moreover, it should be in a
separate patch anyway.

>  
>   /* Domain vCPU flags */
>   REGISTER_LONG_CONSTANT("VIR_DOMAIN_VCPU_CONFIG",
> VIR_DOMAIN_VCPU_CONFIG, CONST_CS | CONST_PERSISTENT);
>   REGISTER_LONG_CONSTANT("VIR_DOMAIN_VCPU_CURRENT",   
> VIR_DOMAIN_VCPU_CURRENT, CONST_CS | CONST_PERSISTENT);
>   REGISTER_LONG_CONSTANT("VIR_DOMAIN_VCPU_LIVE",  
> VIR_DOMAIN_VCPU_LIVE, CONST_CS | CONST_PERSISTENT);
>   REGISTER_LONG_CONSTANT("VIR_DOMAIN_VCPU_MAXIMUM",   
> VIR_DOMAIN_VCPU_MAXIMUM, CONST_CS | CONST_PERSISTENT);
> - REGISTER_LONG_CONSTANT("VIR_DOMAIN_VCPU_GUEST", 
> VIR_DOMAIN_VCPU_GUEST, CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_DOMAIN_VCPU_GUEST", VIR_DOMAIN_VCPU_GUEST, 
> CONST_CS | CONST_PERSISTENT);

So while previously the constants were aligned, now they are not. NACK
to this hunk.

>  
>   #if LIBVIR_VERSION_NUMBER>=8000
>   /* Domain snapshot constants */
>   REGISTER_LONG_CONSTANT("VIR_SNAPSHOT_DELETE_CHILDREN",   
> VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN,   CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_SNAPSHOT_DELETE_METADATA_ONLY", 
> VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY,   CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_SNAPSHOT_DELETE_CHILDREN_ONLY", 
> VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY,   CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_SNAPSHOT_CREATE_REDEFINE",   
> VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE,   CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_SNAPSHOT_CREATE_CURRENT",   
> VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT, CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_SNAPSHOT_CREATE_NO_METADATA",   
> VIR_DOMAIN_SNAPSHOT_CREATE_NO_METADATA, CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_SNAPSHOT_CREATE_HALT",  
> VIR_DOMAIN_SNAPSHOT_CREATE_HALT,CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_SNAPSHOT_CREATE_DISK_ONLY", 
> VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY,   CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_SNAPSHOT_CREATE_REUSE_EXT", 
> VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT,   CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_SNAPSHOT_CREATE_QUIESCE",   
> VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE, CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_SNAPSHOT_CREATE_ATOMIC",
> VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC,  CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_SNAPSHOT_CREATE_LIVE",  
> VIR_DOMAIN_SNAPSHOT_CREATE_LIVE,CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_SNAPSHOT_LIST_DESCENDANTS", 
> VIR_DOMAIN_SNAPSHOT_LIST_DESCENDANTS,   CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_SNAPSHOT_LIST_ROOTS",   
> VIR_DOMAIN_SNAPSHOT_LIST_ROOTS, CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_SNAPSHOT_LIST_METADATA",
> VIR_DOMAIN_SNAPSHOT_LIST_METADATA,  CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_SNAPSHOT_LIST_LEAVES",  
> VIR_DOMAIN_SNAPSHOT_LIST_LEAVES,CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_SNAPSHOT_LIST_NO_LEAVES",   
> VIR_DOMAIN_SNAPSHOT_LIST_NO_LEAVES, CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_SNAPSHOT_LIST_NO_METADATA", 
> VIR_DOMAIN_SNAPSHOT_LIST_NO_METADATA,   CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_

Re: [libvirt] [PATCH v2 16/22] qemu: Refactor qemuMigrationWaitForCompletion

[Peter Krempa]

On Tue, Jun 02, 2015 at 14:34:21 +0200, Jiri Denemark wrote:
> Checking status of all part of migration and aborting it when something
> failed is a complex thing which makes the waiting loop hard to read.
> This patch moves all the checks into a separate function similarly to
> what was done for drive mirror loops.
> 
> Signed-off-by: Jiri Denemark 
> ---
> 

ACK,

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 v2 17/22] qemu_monitor: Wire up MIGRATION event

[Peter Krempa]

On Tue, Jun 02, 2015 at 14:34:22 +0200, Jiri Denemark wrote:
> Thanks to Juan's work QEMU finally emits an event whenever migration
> state changes.
> 
> Signed-off-by: Jiri Denemark 
> ---
> 
> Notes:
> The MIGRATION event is not supported by QEMU yet, this patch is based
> on the current patches available on qemu-devel mailing list. However,
> there were no objections to the design of the event, which makes it
> unlikely to change. Anyway this will have to wait until the patches
> are applied to QEMU.

ACK once this gets accepted to qemu.

> 
> Version 2:
> - new patch

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 v2 02/22] qemu: Introduce qemuBlockJobUpdate

[John Ferlan]

On 06/02/2015 08:34 AM, Jiri Denemark wrote:
> The wrapper is useful for calling qemuBlockJobEventProcess with the
> event details stored in disk's privateData, which is the most likely
> usage of qemuBlockJobEventProcess.
> 
> Signed-off-by: Jiri Denemark 
> ---
> 
> Notes:
> Already ACKed in version 1.
> 
> Version 2:
> - no changes
> 
>  src/libvirt_private.syms |  2 ++
>  src/qemu/qemu_blockjob.c | 37 +
>  src/qemu/qemu_blockjob.h |  3 +++
>  3 files changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 9076135..8846dea 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -265,6 +265,8 @@ virDomainDiskInsert;
>  virDomainDiskInsertPreAlloced;
>  virDomainDiskIoTypeFromString;
>  virDomainDiskIoTypeToString;
> +virDomainDiskMirrorStateTypeFromString;
> +virDomainDiskMirrorStateTypeToString;
>  virDomainDiskPathByName;
>  virDomainDiskRemove;
>  virDomainDiskRemoveByName;
> diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
> index 098a43a..605c2a5 100644
> --- a/src/qemu/qemu_blockjob.c
> +++ b/src/qemu/qemu_blockjob.c
> @@ -38,6 +38,27 @@
>  
>  VIR_LOG_INIT("qemu.qemu_blockjob");
>  
> +
> +int
> +qemuBlockJobUpdate(virQEMUDriverPtr driver,
> +   virDomainObjPtr vm,
> +   virDomainDiskDefPtr disk)
> +{
> +qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
> +int ret;
> +
> +if ((ret = diskPriv->blockJobStatus) == -1)
> +return -1;
> +
> +qemuBlockJobEventProcess(driver, vm, disk,
> + diskPriv->blockJobType,
> + diskPriv->blockJobStatus);
> +diskPriv->blockJobStatus = -1;
> +
> +return ret;
> +}
> +
> +
>  /**
>   * qemuBlockJobEventProcess:
>   * @driver: qemu driver
> @@ -49,8 +70,6 @@ VIR_LOG_INIT("qemu.qemu_blockjob");
>   * Update disk's mirror state in response to a block job event
>   * from QEMU. For mirror state's that must survive libvirt
>   * restart, also update the domain's status XML.
> - *
> - * Returns 0 on success, -1 otherwise.
>   */
>  void
>  qemuBlockJobEventProcess(virQEMUDriverPtr driver,
> @@ -67,6 +86,12 @@ qemuBlockJobEventProcess(virQEMUDriverPtr driver,
>  bool save = false;
>  qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
>  
> +VIR_DEBUG("disk=%s, mirrorState=%s, type=%d, status=%d",
> +  disk->dst,
> +  
> NULLSTR(virDomainDiskMirrorStateTypeToString(disk->mirrorState)),
> +  type,
> +  status);
> +
>  /* Have to generate two variants of the event for old vs. new
>   * client callbacks */
>  if (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT &&
> @@ -218,9 +243,7 @@ qemuBlockJobSyncEnd(virQEMUDriverPtr driver,
>  if (diskPriv->blockJobSync && diskPriv->blockJobStatus != -1) {
>  if (ret_status)
>  *ret_status = diskPriv->blockJobStatus;
> -qemuBlockJobEventProcess(driver, vm, disk,
> - diskPriv->blockJobType,
> - diskPriv->blockJobStatus);
> +qemuBlockJobUpdate(driver, vm, disk);

^^
This doesn't get the returned status...

John
>  diskPriv->blockJobStatus = -1;
>  }
>  diskPriv->blockJobSync = false;
> @@ -300,9 +323,7 @@ qemuBlockJobSyncWaitWithTimeout(virQEMUDriverPtr driver,
>  
>  if (ret_status)
>  *ret_status = diskPriv->blockJobStatus;
> -qemuBlockJobEventProcess(driver, vm, disk,
> - diskPriv->blockJobType,
> - diskPriv->blockJobStatus);
> +qemuBlockJobUpdate(driver, vm, disk);
>  diskPriv->blockJobStatus = -1;
>  
>  return 0;
> diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h
> index ba372a2..81e893e 100644
> --- a/src/qemu/qemu_blockjob.h
> +++ b/src/qemu/qemu_blockjob.h
> @@ -25,6 +25,9 @@
>  # include "internal.h"
>  # include "qemu_conf.h"
>  
> +int qemuBlockJobUpdate(virQEMUDriverPtr driver,
> +   virDomainObjPtr vm,
> +   virDomainDiskDefPtr disk);
>  void qemuBlockJobEventProcess(virQEMUDriverPtr driver,
>virDomainObjPtr vm,
>virDomainDiskDefPtr disk,
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 22/22] qemu: cancel drive mirrors when p2p connection breaks

[Peter Krempa]

On Tue, Jun 02, 2015 at 14:34:27 +0200, Jiri Denemark wrote:
> When a connection to the destination host during a p2p migration drops,
> we know we will have to cancel the migration; it doesn't make sense to
> waste resources by trying to finish the migration. We already do so
> after sending "migrate" command to QEMU and we should do it while
> waiting for drive mirrors to become ready too.
> 
> Signed-off-by: Jiri Denemark 
> ---
> 

ACK,

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 v2 02/22] qemu: Introduce qemuBlockJobUpdate

[Jiri Denemark]

On Wed, Jun 10, 2015 at 07:21:27 -0400, John Ferlan wrote:
> 
> 
> On 06/02/2015 08:34 AM, Jiri Denemark wrote:
> > The wrapper is useful for calling qemuBlockJobEventProcess with the
> > event details stored in disk's privateData, which is the most likely
> > usage of qemuBlockJobEventProcess.
> > 
> > Signed-off-by: Jiri Denemark 
> > ---
...
> > diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
> > index 098a43a..605c2a5 100644
> > --- a/src/qemu/qemu_blockjob.c
> > +++ b/src/qemu/qemu_blockjob.c
...
> > @@ -218,9 +243,7 @@ qemuBlockJobSyncEnd(virQEMUDriverPtr driver,
> >  if (diskPriv->blockJobSync && diskPriv->blockJobStatus != -1) {
> >  if (ret_status)
> >  *ret_status = diskPriv->blockJobStatus;
> > -qemuBlockJobEventProcess(driver, vm, disk,
> > - diskPriv->blockJobType,
> > - diskPriv->blockJobStatus);
> > +qemuBlockJobUpdate(driver, vm, disk);
> 
> ^^
> This doesn't get the returned status...

qemuBlockJobUpdate returns the original value of
diskPriv->blockJobStatus, which is already stored to *ret_status above.
Not to mention that one of the following patches will completely remove
ret_status from qemuBlockJobSyncEnd.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [sandbox PATCH 0/3] Disk support for libvirt-sandbox

[Eren Yagdiran]

Hello,

These patches provide disk support for libvirt-sandbox.
Implemented '--disk' parameter will be useful when integrating Docker image 
support for libvirt-sandbox.
Currently, we only support qcow2 file format and fallback is set to RAW type.

Eren Yagdiran (3):
  Add an utility function for guessing filetype from file extension
  Add configuration object for disk support
  Add disk parameter to virt-sandbox

 bin/virt-sandbox.c |  37 +++
 libvirt-sandbox/Makefile.am|   3 +
 .../libvirt-sandbox-builder-container.c|  36 ++-
 libvirt-sandbox/libvirt-sandbox-builder-machine.c  |  31 ++-
 libvirt-sandbox/libvirt-sandbox-config-disk.c  | 300 +
 libvirt-sandbox/libvirt-sandbox-config-disk.h  |  86 ++
 libvirt-sandbox/libvirt-sandbox-config.c   | 275 +++
 libvirt-sandbox/libvirt-sandbox-config.h   |  10 +
 libvirt-sandbox/libvirt-sandbox-util.c |  79 ++
 libvirt-sandbox/libvirt-sandbox-util.h |   6 +
 libvirt-sandbox/libvirt-sandbox.h  |   1 +
 libvirt-sandbox/libvirt-sandbox.sym|   8 +
 libvirt-sandbox/tests/test-config.c|  11 +
 13 files changed, 881 insertions(+), 2 deletions(-)
 create mode 100644 libvirt-sandbox/libvirt-sandbox-config-disk.c
 create mode 100644 libvirt-sandbox/libvirt-sandbox-config-disk.h
 create mode 100644 libvirt-sandbox/libvirt-sandbox-util.c

-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [sandbox PATCH 1/3] Add an utility function for guessing filetype from file extension

[Eren Yagdiran]

Consider the file name extension as the image type, except for .img that are 
usually RAW images
---
 libvirt-sandbox/Makefile.am|  1 +
 libvirt-sandbox/libvirt-sandbox-util.c | 79 ++
 libvirt-sandbox/libvirt-sandbox-util.h |  6 +++
 3 files changed, 86 insertions(+)
 create mode 100644 libvirt-sandbox/libvirt-sandbox-util.c

diff --git a/libvirt-sandbox/Makefile.am b/libvirt-sandbox/Makefile.am
index 96302cb..6917f04 100644
--- a/libvirt-sandbox/Makefile.am
+++ b/libvirt-sandbox/Makefile.am
@@ -84,6 +84,7 @@ SANDBOX_HEADER_FILES = \
$(NULL)
 SANDBOX_SOURCE_FILES = \
libvirt-sandbox-main.c \
+   libvirt-sandbox-util.c \
libvirt-sandbox-config.c \
libvirt-sandbox-config-network.c \
libvirt-sandbox-config-network-address.c \
diff --git a/libvirt-sandbox/libvirt-sandbox-util.c 
b/libvirt-sandbox/libvirt-sandbox-util.c
new file mode 100644
index 000..0ab4fac
--- /dev/null
+++ b/libvirt-sandbox/libvirt-sandbox-util.c
@@ -0,0 +1,79 @@
+/*
+ * libvirt-sandbox-util.c: libvirt sandbox util functions
+ *
+ * Copyright (C) 2015 Universitat Polit??cnica de Catalunya.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  
USA
+ *
+ * Author: Eren Yagdiran 
+ */
+
+#include 
+#include 
+
+#include "libvirt-sandbox/libvirt-sandbox.h"
+
+/* This array contains string values for GVirConfigDomainDiskFormat,
+ * order is important.*/
+static const gchar *FORMATS_STRINGS[] = {
+"raw",
+"dir",
+"bochs",
+"cloop",
+"cow",
+"dmg",
+"iso",
+"qcow",
+"qcow2",
+"qed",
+"vmdk",
+"vpc",
+"fat",
+"vhd",
+NULL
+};
+
+gint gvir_sandbox_util_guess_image_format(const gchar *path){
+
+gchar *tmp;
+
+if ((tmp = strchr(path, '.')) == NULL) {
+return -1;
+}
+tmp = tmp + 1;
+
+if (strcmp(tmp,"img")==0){
+   return GVIR_CONFIG_DOMAIN_DISK_FORMAT_RAW;
+}
+
+return gvir_sandbox_util_disk_format_from_str(tmp);
+}
+
+gint gvir_sandbox_util_disk_format_from_str(const gchar *value)
+{
+gint i = 0;
+
+while (FORMATS_STRINGS[i] != NULL) {
+   if (strcmp(FORMATS_STRINGS[i], value) == 0)
+return i;
+i++;
+}
+return -1;
+}
+
+const gchar *gvir_sandbox_util_disk_format_to_str(GVirConfigDomainDiskFormat 
format)
+{
+return FORMATS_STRINGS[format];
+}
diff --git a/libvirt-sandbox/libvirt-sandbox-util.h 
b/libvirt-sandbox/libvirt-sandbox-util.h
index ae6b74b..fbd3785 100644
--- a/libvirt-sandbox/libvirt-sandbox-util.h
+++ b/libvirt-sandbox/libvirt-sandbox-util.h
@@ -29,6 +29,12 @@
 
 G_BEGIN_DECLS
 
+gint gvir_sandbox_util_guess_image_format(const gchar *path);
+
+const gchar *gvir_sandbox_util_disk_format_to_str(GVirConfigDomainDiskFormat 
format);
+
+gint gvir_sandbox_util_disk_format_from_str(const gchar *value);
+
 /**
  * LIBVIRT_SANDBOX_CLASS_PADDING: (skip)
  */
-- 
2.1.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [sandbox PATCH 3/3] Add disk parameter to virt-sandbox

[Eren Yagdiran]

Allow users to add disk images to their sandbox. Only disk images are supported 
so far, but the
parameter is intentionally designed for future changes.
---
 bin/virt-sandbox.c | 37 ++
 .../libvirt-sandbox-builder-container.c| 36 -
 libvirt-sandbox/libvirt-sandbox-builder-machine.c  | 31 +-
 3 files changed, 102 insertions(+), 2 deletions(-)

diff --git a/bin/virt-sandbox.c b/bin/virt-sandbox.c
index 1ab2f8a..aa09f33 100644
--- a/bin/virt-sandbox.c
+++ b/bin/virt-sandbox.c
@@ -63,6 +63,7 @@ int main(int argc, char **argv) {
 GMainLoop *loop = NULL;
 GError *error = NULL;
 gchar *name = NULL;
+gchar **disks = NULL;
 gchar **mounts = NULL;
 gchar **includes = NULL;
 gchar *includefile = NULL;
@@ -92,6 +93,8 @@ int main(int argc, char **argv) {
   N_("name of the sandbox"), "NAME" },
 { "root", 'r', 0, G_OPTION_ARG_STRING, &root,
   N_("root directory of the sandbox"), "DIR" },
+{ "disk", ' ', 0, G_OPTION_ARG_STRING_ARRAY, &disks,
+  N_("add a disk in the guest"), "TYPE:TARGET=SOURCE,FORMAT=FORMAT" },
 { "mount", 'm', 0, G_OPTION_ARG_STRING_ARRAY, &mounts,
   N_("mount a filesystem in the guest"), "TYPE:TARGET=SOURCE" },
 { "include", 'i', 0, G_OPTION_ARG_STRING_ARRAY, &includes,
@@ -182,6 +185,13 @@ int main(int argc, char **argv) {
 gvir_sandbox_config_set_username(cfg, "root");
 }
 
+if (disks &&
+!gvir_sandbox_config_add_disk_strv(cfg, disks, &error)) {
+g_printerr(_("Unable to parse disks: %s\n"),
+   error && error->message ? error->message : _("Unknown 
failure"));
+goto cleanup;
+}
+
 if (mounts &&
 !gvir_sandbox_config_add_mount_strv(cfg, mounts, &error)) {
 g_printerr(_("Unable to parse mounts: %s\n"),
@@ -319,6 +329,33 @@ inheriting the host's root filesystem.
 NB. C must contain a matching install of the libvirt-sandbox
 package. This restriction may be lifted in a future version.
 
+=item B<--disk TYPE:TARGET=SOURCE,FORMAT=FORMAT>
+
+Sets up a disk inside the sandbox by using B with target device 
B
+and type B and format B. Example: 
file:hda=/var/lib/sandbox/demo/tmp.qcow2,format=qcow2
+Format is an optional parameter.
+
+=over 4
+
+=item B
+
+Type parameter can be set to "file".
+
+=item B
+
+Target parameter can be set to "hda" or any other libvirt supported target disk
+
+=item B
+
+Source parameter needs to point a file which must be a one of the valid domain 
disk formats supported by qemu.
+
+=item B
+
+Format parameter must be set to the same disk format as the file passed on 
source parameter. 
+This parameter is optional and the format can be guessed from the image 
extension
+
+=back
+
 =item B<-m TYPE:DST=SRC>, B<--mount TYPE:DST=SRC>
 
 Sets up a mount inside the sandbox at B backed by B. The
diff --git a/libvirt-sandbox/libvirt-sandbox-builder-container.c 
b/libvirt-sandbox/libvirt-sandbox-builder-container.c
index c3a58b2..9839810 100644
--- a/libvirt-sandbox/libvirt-sandbox-builder-container.c
+++ b/libvirt-sandbox/libvirt-sandbox-builder-container.c
@@ -218,7 +218,9 @@ static gboolean 
gvir_sandbox_builder_container_construct_devices(GVirSandboxBuil
 GVirConfigDomainInterfaceNetwork *iface;
 GVirConfigDomainConsole *con;
 GVirConfigDomainChardevSourcePty *src;
-GList *tmp = NULL, *mounts = NULL, *networks = NULL;
+GVirConfigDomainDisk *disk;
+GVirConfigDomainDiskDriver *diskDriver;
+GList *tmp = NULL, *mounts = NULL, *networks = NULL, *disks = NULL;
 gchar *configdir = g_strdup_printf("%s/config", statedir);
 gboolean ret = FALSE;
 
@@ -226,6 +228,38 @@ static gboolean 
gvir_sandbox_builder_container_construct_devices(GVirSandboxBuil
 construct_devices(builder, config, statedir, domain, error))
 goto cleanup;
 
+
+tmp = disks = gvir_sandbox_config_get_disks(config);
+while(tmp){
+GVirSandboxConfigDisk *dconfig = GVIR_SANDBOX_CONFIG_DISK(tmp->data);
+
+if (GVIR_SANDBOX_IS_CONFIG_DISK(dconfig)){
+
+disk = gvir_config_domain_disk_new();
+diskDriver = gvir_config_domain_disk_driver_new();
+gvir_config_domain_disk_set_type(disk,
+ 
gvir_sandbox_config_disk_get_disk_type(dconfig));
+gvir_config_domain_disk_driver_set_format(diskDriver,
+  
gvir_sandbox_config_disk_get_format(dconfig));
+gvir_config_domain_disk_driver_set_name(diskDriver, "nbd");
+gvir_config_domain_disk_set_source(disk,
+   
gvir_sandbox_config_disk_get_source(dconfig));
+gvir_config_domain_disk_set_target_bus(disk,
+   
GVIR_CONFIG_DOMAIN_DISK_BUS_IDE);
+gvir_config_domain_disk_set_target_dev(disk

Re: [libvirt] [PATCH v2 02/22] qemu: Introduce qemuBlockJobUpdate

[John Ferlan]

On 06/10/2015 07:31 AM, Jiri Denemark wrote:
> On Wed, Jun 10, 2015 at 07:21:27 -0400, John Ferlan wrote:
>>
>>
>> On 06/02/2015 08:34 AM, Jiri Denemark wrote:
>>> The wrapper is useful for calling qemuBlockJobEventProcess with the
>>> event details stored in disk's privateData, which is the most likely
>>> usage of qemuBlockJobEventProcess.
>>>
>>> Signed-off-by: Jiri Denemark 
>>> ---
> ...
>>> diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
>>> index 098a43a..605c2a5 100644
>>> --- a/src/qemu/qemu_blockjob.c
>>> +++ b/src/qemu/qemu_blockjob.c
> ...
>>> @@ -218,9 +243,7 @@ qemuBlockJobSyncEnd(virQEMUDriverPtr driver,
>>>  if (diskPriv->blockJobSync && diskPriv->blockJobStatus != -1) {
>>>  if (ret_status)
>>>  *ret_status = diskPriv->blockJobStatus;
>>> -qemuBlockJobEventProcess(driver, vm, disk,
>>> - diskPriv->blockJobType,
>>> - diskPriv->blockJobStatus);
>>> +qemuBlockJobUpdate(driver, vm, disk);
>>
>> ^^
>> This doesn't get the returned status...
> 
> qemuBlockJobUpdate returns the original value of
> diskPriv->blockJobStatus, which is already stored to *ret_status above.
> Not to mention that one of the following patches will completely remove
> ret_status from qemuBlockJobSyncEnd.
> 

I was just perusing to 'learn' a bit about the code... I hadn't looked
forward yet.

In essence it's a void function though... I see that qemuBlockJobUpdate
checks/sets blockJobStatus = -1 and the same setting is done right after
the call.

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [sandbox PATCH 2/3] Add configuration object for disk support

[Eren Yagdiran]

Add the config gobject, functions to store and load the new configuration
fragments and test. This will allow creating sandboxes with attached
disk with a parameter formatted like file:hda=/source/file.qcow2,format=qcow2
---
 libvirt-sandbox/Makefile.am   |   2 +
 libvirt-sandbox/libvirt-sandbox-config-disk.c | 300 ++
 libvirt-sandbox/libvirt-sandbox-config-disk.h |  86 
 libvirt-sandbox/libvirt-sandbox-config.c  | 275 +++
 libvirt-sandbox/libvirt-sandbox-config.h  |  10 +
 libvirt-sandbox/libvirt-sandbox.h |   1 +
 libvirt-sandbox/libvirt-sandbox.sym   |   8 +
 libvirt-sandbox/tests/test-config.c   |  11 +
 8 files changed, 693 insertions(+)
 create mode 100644 libvirt-sandbox/libvirt-sandbox-config-disk.c
 create mode 100644 libvirt-sandbox/libvirt-sandbox-config-disk.h

diff --git a/libvirt-sandbox/Makefile.am b/libvirt-sandbox/Makefile.am
index 6917f04..8237c50 100644
--- a/libvirt-sandbox/Makefile.am
+++ b/libvirt-sandbox/Makefile.am
@@ -55,6 +55,7 @@ SANDBOX_HEADER_FILES = \
libvirt-sandbox-main.h \
libvirt-sandbox-util.h \
libvirt-sandbox-config.h \
+   libvirt-sandbox-config-disk.h \
libvirt-sandbox-config-network.h \
libvirt-sandbox-config-network-address.h \
libvirt-sandbox-config-network-filterref-parameter.h \
@@ -86,6 +87,7 @@ SANDBOX_SOURCE_FILES = \
libvirt-sandbox-main.c \
libvirt-sandbox-util.c \
libvirt-sandbox-config.c \
+   libvirt-sandbox-config-disk.c \
libvirt-sandbox-config-network.c \
libvirt-sandbox-config-network-address.c \
libvirt-sandbox-config-network-filterref.c \
diff --git a/libvirt-sandbox/libvirt-sandbox-config-disk.c 
b/libvirt-sandbox/libvirt-sandbox-config-disk.c
new file mode 100644
index 000..75d7557
--- /dev/null
+++ b/libvirt-sandbox/libvirt-sandbox-config-disk.c
@@ -0,0 +1,300 @@
+/*
+ * libvirt-sandbox-config-disk.c: libvirt sandbox configuration
+ *
+ * Copyright (C) 2015 Universitat Polit??cnica de Catalunya.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  
USA
+ *
+ * Author: Eren Yagdiran 
+ */
+
+#include 
+#include 
+
+#include "libvirt-sandbox/libvirt-sandbox.h"
+
+/**
+ * SECTION: libvirt-sandbox-config-disk
+ * @short_description: Disk attachment configuration details
+ * @include: libvirt-sandbox/libvirt-sandbox.h
+ * @see_aloso: #GVirSandboxConfig
+ *
+ * Provides an object to store information about a disk attachment in the 
sandbox
+ *
+ */
+
+#define GVIR_SANDBOX_CONFIG_DISK_GET_PRIVATE(obj)  \
+(G_TYPE_INSTANCE_GET_PRIVATE((obj), GVIR_SANDBOX_TYPE_CONFIG_DISK, 
GVirSandboxConfigDiskPrivate))
+
+/* This array contains string values for GVirConfigDomainDiskType,
+ * order is important.*/
+static const gchar *TYPES_STRINGS[] = {
+"file",
+"block",
+"dir",
+"network",
+NULL
+};
+
+struct _GVirSandboxConfigDiskPrivate
+{
+GVirConfigDomainDiskType type;
+gchar *target;
+gchar *source;
+GVirConfigDomainDiskFormat format;
+};
+
+G_DEFINE_TYPE(GVirSandboxConfigDisk, gvir_sandbox_config_disk, G_TYPE_OBJECT);
+
+
+enum {
+PROP_0,
+PROP_TYPE,
+PROP_TARGET,
+PROP_SOURCE,
+PROP_FORMAT
+};
+
+enum {
+LAST_SIGNAL
+};
+
+
+
+static void gvir_sandbox_config_disk_get_property(GObject *object,
+  guint prop_id,
+  GValue *value,
+  GParamSpec *pspec)
+{
+GVirSandboxConfigDisk *config = GVIR_SANDBOX_CONFIG_DISK(object);
+GVirSandboxConfigDiskPrivate *priv = config->priv;
+
+switch (prop_id) {
+case PROP_TARGET:
+g_value_set_string(value, priv->target);
+break;
+case PROP_SOURCE:
+g_value_set_string(value, priv->source);
+break;
+case PROP_TYPE:
+g_value_set_enum(value, priv->type);
+break;
+case PROP_FORMAT:
+g_value_set_enum(value, priv->for

Re: [libvirt] [PATCH v2 04/22] qemu: Use domain condition for synchronous block jobs

[John Ferlan]

On 06/02/2015 08:34 AM, Jiri Denemark wrote:
> By switching block jobs to use domain conditions, we can drop some
> pretty complicated code in NBD storage migration. Moreover, we are
> getting ready for migration events (to replace our 50ms polling on
> query-migrate). The ultimate goal is to have a single loop waiting
> (virDomainObjWait) for any migration related event (changed status of a
> migration, disk mirror events, internal abort requests, ...). This patch
> makes NBD storage migration ready for this: first we call a QMP command
> to start or cancel drive mirror on all disks we are interested in and
> then we wait for a single condition which is signaled on any event
> related to any of the mirrors.
> 
> Signed-off-by: Jiri Denemark 
> ---
> 
> Notes:
> Version 2:
> - slightly modified to use domain conditions
> 
>  po/POTFILES.in|   1 -
>  src/qemu/qemu_blockjob.c  | 137 ++---
>  src/qemu/qemu_blockjob.h  |  12 +-
>  src/qemu/qemu_domain.c|  17 +--
>  src/qemu/qemu_domain.h|   1 -
>  src/qemu/qemu_driver.c|  24 ++--
>  src/qemu/qemu_migration.c | 299 
> ++
>  src/qemu/qemu_process.c   |  13 +-
>  8 files changed, 197 insertions(+), 307 deletions(-)
> 

...


> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 93e29e7..61b3e34 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -1720,7 +1720,7 @@ qemuMigrationStopNBDServer(virQEMUDriverPtr driver,
>  
>  
>  /**
> - * qemuMigrationCheckDriveMirror:
> + * qemuMigrationDriveMirrorReady:
>   * @driver: qemu driver
>   * @vm: domain
>   *
> @@ -1733,111 +1733,148 @@ qemuMigrationStopNBDServer(virQEMUDriverPtr driver,
>   *-1 on error.
>   */
>  static int
> -qemuMigrationCheckDriveMirror(virQEMUDriverPtr driver,
> +qemuMigrationDriveMirrorReady(virQEMUDriverPtr driver,
>virDomainObjPtr vm)
>  {
>  size_t i;
> -int ret = 1;
> +size_t notReady = 0;
> +int status;
>  
>  for (i = 0; i < vm->def->ndisks; i++) {
>  virDomainDiskDefPtr disk = vm->def->disks[i];
>  qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
>  
> -if (!diskPriv->migrating || !diskPriv->blockJobSync)
> +if (!diskPriv->migrating)
>  continue;
>  
> -/* process any pending event */
> -if (qemuBlockJobSyncWaitWithTimeout(driver, vm, disk,
> -0ull, NULL) < 0)
> -return -1;
> -
> -switch (disk->mirrorState) {
> -case VIR_DOMAIN_DISK_MIRROR_STATE_NONE:
> -ret = 0;
> -break;
> -case VIR_DOMAIN_DISK_MIRROR_STATE_ABORT:
> +status = qemuBlockJobUpdate(driver, vm, disk);
> +if (status == VIR_DOMAIN_BLOCK_JOB_FAILED) {

ah - and now I see it cannot be a void function; however, this could
cause Coverity checker to complain about other uses.  I wasn't able to
apply the series to my Coverity branch since it wouldn't git am apply.

Still that leaves me wondering why other paths don't check the status.
They may not care, but with one or more paths checking (as seen in the
rest of this module) it leaves a future reader not knowing whether the
paths that aren't checking should check.

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 19/22] qemu: Work around weired migration status changes

[Peter Krempa]

On Tue, Jun 02, 2015 at 14:34:24 +0200, Jiri Denemark wrote:
> When cancelling migration we can see the following conversation on QMP
> monitor:
> 
> {"execute":"migrate_cancel","id":"libvirt-33"}
> {"timestamp": {"seconds": 1432899178, "microseconds": 844907}, "event":
> "MIGRATION", "data": {"status": "cancelling"}}
> {"return": {}, "id": "libvirt-33"}
> {"timestamp": {"seconds": 1432899178, "microseconds": 845625}, "event":
> "MIGRATION", "data": {"status": "failed"}}
> {"timestamp": {"seconds": 1432899178, "microseconds": 846432}, "event":
> "MIGRATION", "data": {"status": "cancelled"}}
> 
> That is, migration status first changes to "failed" just to change to
> the correct "cancelled" state in a few moments later. However, this is
> enough to let libvirt report migration failed for unknown reason instead
> of having been cancelled by a user.
> 
> This should really be fixed in QEMU but I'm not sure how easy it is.

Is qemu going to fix it? I'd rather see it done that way rather than
fixing it up in libvirt.



signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 20/22] qemuDomainGetJobStatsInternal: Support migration events

[Peter Krempa]

On Tue, Jun 02, 2015 at 14:34:25 +0200, Jiri Denemark wrote:
> When QEMU supports migration events qemuDomainJobInfo structure is no
> longer updated with migration statistics. We have to enter a job and
> explicitly ask QEMU every time virDomainGetJob{Info,Stats} is called.
> 
> Signed-off-by: Jiri Denemark 
> ---

ACK, although this patch should go before patch 18 of this series.

Peter


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [sandbox PATCH 0/3] Disk support for libvirt-sandbox

[Daniel P. Berrange]

On Wed, Jun 10, 2015 at 01:40:07PM +0200, Eren Yagdiran wrote:
> Hello,
> 
> These patches provide disk support for libvirt-sandbox.
> Implemented '--disk' parameter will be useful when integrating Docker image 
> support for libvirt-sandbox.
> Currently, we only support qcow2 file format and fallback is set to RAW type.

Can you explain a bit more why you need to be able to expose a virtual
disk to the sandbox. If the stuff running inside the sandbox is not
privileged, it won't even have access to the device node inside it,
nor be able to mount it. This is why the sandbox code focuses on
mounting everything itself before handing off the main app to run.
So I'm not really clear how this feature is going to be generally
useful or usable.


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 v2 21/22] qemu: Wait for migration events on domain condition

[Peter Krempa]

On Tue, Jun 02, 2015 at 14:34:26 +0200, Jiri Denemark wrote:
> Since we already support the MIGRATION event, we just need to make sure
> the domain condition is signalled whenever a p2p connection drops or the
> domain is paused due to IO error and we can avoid waking up every 50 ms
> to check whether something happened.
> 
> Signed-off-by: Jiri Denemark 
> ---
>

ACK,

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 v2 18/22] qemu: Update migration state according to MIGRATION event

[Peter Krempa]

On Tue, Jun 02, 2015 at 14:34:23 +0200, Jiri Denemark wrote:
> We don't need to call query-migrate every 50ms when we get the current
> migration state via MIGRATION event.
> 
> Signed-off-by: Jiri Denemark 
> ---

ACK,

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 v2 04/22] qemu: Use domain condition for synchronous block jobs

[Jiri Denemark]

On Wed, Jun 10, 2015 at 07:56:32 -0400, John Ferlan wrote:
> 
> 
> On 06/02/2015 08:34 AM, Jiri Denemark wrote:
> > By switching block jobs to use domain conditions, we can drop some
> > pretty complicated code in NBD storage migration. Moreover, we are
> > getting ready for migration events (to replace our 50ms polling on
> > query-migrate). The ultimate goal is to have a single loop waiting
> > (virDomainObjWait) for any migration related event (changed status of a
> > migration, disk mirror events, internal abort requests, ...). This patch
> > makes NBD storage migration ready for this: first we call a QMP command
> > to start or cancel drive mirror on all disks we are interested in and
> > then we wait for a single condition which is signaled on any event
> > related to any of the mirrors.
> > 
> > Signed-off-by: Jiri Denemark 
> > ---
> > 
> > Notes:
> > Version 2:
> > - slightly modified to use domain conditions
> > 
> >  po/POTFILES.in|   1 -
> >  src/qemu/qemu_blockjob.c  | 137 ++---
> >  src/qemu/qemu_blockjob.h  |  12 +-
> >  src/qemu/qemu_domain.c|  17 +--
> >  src/qemu/qemu_domain.h|   1 -
> >  src/qemu/qemu_driver.c|  24 ++--
> >  src/qemu/qemu_migration.c | 299 
> > ++
> >  src/qemu/qemu_process.c   |  13 +-
> >  8 files changed, 197 insertions(+), 307 deletions(-)
> > 
> 
> ...
> 
> 
> > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> > index 93e29e7..61b3e34 100644
> > --- a/src/qemu/qemu_migration.c
> > +++ b/src/qemu/qemu_migration.c
> > @@ -1720,7 +1720,7 @@ qemuMigrationStopNBDServer(virQEMUDriverPtr driver,
> >  
> >  
> >  /**
> > - * qemuMigrationCheckDriveMirror:
> > + * qemuMigrationDriveMirrorReady:
> >   * @driver: qemu driver
> >   * @vm: domain
> >   *
> > @@ -1733,111 +1733,148 @@ qemuMigrationStopNBDServer(virQEMUDriverPtr 
> > driver,
> >   *-1 on error.
> >   */
> >  static int
> > -qemuMigrationCheckDriveMirror(virQEMUDriverPtr driver,
> > +qemuMigrationDriveMirrorReady(virQEMUDriverPtr driver,
> >virDomainObjPtr vm)
> >  {
> >  size_t i;
> > -int ret = 1;
> > +size_t notReady = 0;
> > +int status;
> >  
> >  for (i = 0; i < vm->def->ndisks; i++) {
> >  virDomainDiskDefPtr disk = vm->def->disks[i];
> >  qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
> >  
> > -if (!diskPriv->migrating || !diskPriv->blockJobSync)
> > +if (!diskPriv->migrating)
> >  continue;
> >  
> > -/* process any pending event */
> > -if (qemuBlockJobSyncWaitWithTimeout(driver, vm, disk,
> > -0ull, NULL) < 0)
> > -return -1;
> > -
> > -switch (disk->mirrorState) {
> > -case VIR_DOMAIN_DISK_MIRROR_STATE_NONE:
> > -ret = 0;
> > -break;
> > -case VIR_DOMAIN_DISK_MIRROR_STATE_ABORT:
> > +status = qemuBlockJobUpdate(driver, vm, disk);
> > +if (status == VIR_DOMAIN_BLOCK_JOB_FAILED) {
> 
> ah - and now I see it cannot be a void function; however, this could
> cause Coverity checker to complain about other uses.  I wasn't able to
> apply the series to my Coverity branch since it wouldn't git am apply.
> 
> Still that leaves me wondering why other paths don't check the status.
> They may not care, but with one or more paths checking (as seen in the
> rest of this module) it leaves a future reader not knowing whether the
> paths that aren't checking should check.

Well, the return value is the event qemuBlockJobUpdate processed, it's
not indicating success or failure, qemuBlockJobUpdate just always
succeeds. So I think it's pretty clear that if the caller is not
interested in the event which got processed (most callers just check the
resulting state), it just ignores it. We can always add ignore_valu() if
coverity is not happy about it...

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] virNumaSetPagePoolSize: Produce friendlier error message

[Martin Kletzander]

On Wed, Jun 10, 2015 at 11:16:38AM +0200, Michal Privoznik wrote:

On 09.06.2015 08:42, Martin Kletzander wrote:

On Mon, Jun 08, 2015 at 02:41:05PM +0200, Michal Privoznik wrote:

https://bugzilla.redhat.com/show_bug.cgi?id=1224587

The function takes two important arguments (among many others): @node
and @page_size. From these two a path under /sys is constructed. The
path is then used to read and write the desired size of huge pages
pool. However, if the path does not exists due to either @node or
@page_size having nonexistent value (e.g. there's no such NUMA node or
no page size like -2), an cryptic error message is produced:

 virsh # allocpages --pagesize 2049 --pagecount 8 --cellno -2
 error: Failed to open file
'/sys/devices/system/node/node-2/hugepages/hugepages-2049kB/nr_hugepages':
No such file or directory

Add two more checks to catch this and therefore produce much more
friendlier error messages.

Signed-off-by: Michal Privoznik 
---
src/util/virnuma.c | 18 ++
1 file changed, 18 insertions(+)

diff --git a/src/util/virnuma.c b/src/util/virnuma.c
index 669192a..5807d8f 100644
--- a/src/util/virnuma.c
+++ b/src/util/virnuma.c
@@ -849,9 +849,27 @@ virNumaSetPagePoolSize(int node,
goto cleanup;
}

+if (node != -1 && !virNumaNodeIsAvailable(node)) {
+virReportError(VIR_ERR_OPERATION_FAILED,
+   _("NUMA node %d is not available"),
+   node);
+goto cleanup;
+}
+
if (virNumaGetHugePageInfoPath(&nr_path, node, page_size,
"nr_hugepages") < 0)
goto cleanup;

+if (!virFileExists(nr_path)) {
+/* Strictly speaking, @nr_path contains both NUMA node and
page size.
+ * So if it doesn't exist it can be due to any of those two
is wrong.
+ * However, the existence of the node was checked a few lines
above, so
+ * it can be only page size here. */


Über-strictly speaking, unless you compile with both WITH_NUMACTL &&
HAVE_NUMA_BITMASK_ISBITSET then virNumaNodeIsAvailable() can pass for
invalid node in case of non-contiguous NUMA node numbers.


So what are you saying is that I should update the comment or the error
message or leave everything as-is and push it?



It might've came out too strong.  I just meant that the error message
virFileReadAll would've gave sounds enough to me, but if you want to
add this here, then I'd suggest mentioning the NUMA node there as
well.

However, feel free to correct me as I might misunderstood some higher
purpose you had in mind; that's why I wanted to initiate a possible
discussion about it if that was the case.

Long story short, I'd say ACK with updated error message.

Martin


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 19/22] qemu: Work around weired migration status changes

[Jiri Denemark]

On Wed, Jun 10, 2015 at 13:06:03 +0200, Peter Krempa wrote:
> On Tue, Jun 02, 2015 at 14:34:24 +0200, Jiri Denemark wrote:
> > When cancelling migration we can see the following conversation on QMP
> > monitor:
> > 
> > {"execute":"migrate_cancel","id":"libvirt-33"}
> > {"timestamp": {"seconds": 1432899178, "microseconds": 844907}, "event":
> > "MIGRATION", "data": {"status": "cancelling"}}
> > {"return": {}, "id": "libvirt-33"}
> > {"timestamp": {"seconds": 1432899178, "microseconds": 845625}, "event":
> > "MIGRATION", "data": {"status": "failed"}}
> > {"timestamp": {"seconds": 1432899178, "microseconds": 846432}, "event":
> > "MIGRATION", "data": {"status": "cancelled"}}
> > 
> > That is, migration status first changes to "failed" just to change to
> > the correct "cancelled" state in a few moments later. However, this is
> > enough to let libvirt report migration failed for unknown reason instead
> > of having been cancelled by a user.
> > 
> > This should really be fixed in QEMU but I'm not sure how easy it is.
> 
> Is qemu going to fix it? I'd rather see it done that way rather than
> fixing it up in libvirt.

Yeah, Juan said he would fix it in the next version of his series.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] add some more missing libvirt functions

[Michal Privoznik]

On 10.06.2015 10:18, Vasiliy Tolstov wrote:
> * libvirt_connect_get_all_domain_stats
> * libvirt_domain_block_resize
> * libvirt_domain_block_job_abort
> * libvirt_domain_block_job_set_speed
> 
> Signed-off-by: Vasiliy Tolstov 
> ---
>  src/libvirt-php.c | 177 
> +-
>  src/libvirt-php.h |   4 ++
>  2 files changed, 180 insertions(+), 1 deletion(-)

>From the e-mail header:

Content-Type: text/plain; charset=yes

I've not know there's such charset as yes :)


> 
> diff --git a/src/libvirt-php.c b/src/libvirt-php.c
> index e9b9657..f9096ef 100644
> --- a/src/libvirt-php.c
> +++ b/src/libvirt-php.c
> @@ -91,6 +91,7 @@ static zend_function_entry libvirt_functions[] = {
>   PHP_FE(libvirt_connect_get_maxvcpus, NULL)
>   PHP_FE(libvirt_connect_get_encrypted, NULL)
>   PHP_FE(libvirt_connect_get_secure, NULL)
> + PHP_FE(libvirt_connect_get_all_domain_stats, NULL)
>   /* Stream functions */
>   PHP_FE(libvirt_stream_create, NULL)
>   PHP_FE(libvirt_stream_close, NULL)
> @@ -136,6 +137,10 @@ static zend_function_entry libvirt_functions[] = {
>   PHP_FE(libvirt_domain_memory_peek,NULL)
>   PHP_FE(libvirt_domain_memory_stats,NULL)
>   PHP_FE(libvirt_domain_block_stats,NULL)
> + PHP_FE(libvirt_domain_block_resize,NULL)
> + //  PHP_FE(libvirt_domain_block_copy,NULL)

Just drop this line.

> + PHP_FE(libvirt_domain_block_job_abort,NULL)
> + PHP_FE(libvirt_domain_block_job_set_speed,NULL)
>   PHP_FE(libvirt_domain_interface_stats,NULL)
>   PHP_FE(libvirt_domain_get_connect, NULL)
>   PHP_FE(libvirt_domain_migrate, NULL)
> @@ -1332,6 +1337,11 @@ PHP_MINIT_FUNCTION(libvirt)
>   /* Job was aborted but it's not cleanup up yet */
>   REGISTER_LONG_CONSTANT("VIR_DOMAIN_JOB_CANCELLED",  5, CONST_CS | 
> CONST_PERSISTENT);
>  
> + REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC",  
> VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC, CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT",  
> VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT, CONST_CS | CONST_PERSISTENT);
> +
> + REGISTER_LONG_CONSTANT("VIR_DOMAIN_BLOCK_JOB_SPEED_BANDWIDTH_BYTES",
> VIR_DOMAIN_BLOCK_JOB_SPEED_BANDWIDTH_BYTES, CONST_CS | CONST_PERSISTENT);
> +
>   /* Migration constants */
>   REGISTER_LONG_CONSTANT("VIR_MIGRATE_LIVE",1, CONST_CS | 
> CONST_PERSISTENT);
>   /* direct source -> dest host control channel Note the less-common 
> spelling that we're stuck with: */
> @@ -1374,7 +1384,7 @@ PHP_MINIT_FUNCTION(libvirt)
>   REGISTER_LONG_CONSTANT("VIR_DOMAIN_FLAG_TEST_LOCAL_VNC",
> DOMAIN_FLAG_TEST_LOCAL_VNC, CONST_CS | CONST_PERSISTENT);
>   REGISTER_LONG_CONSTANT("VIR_DOMAIN_FLAG_SOUND_AC97",
> DOMAIN_FLAG_SOUND_AC97, CONST_CS | CONST_PERSISTENT);
>   REGISTER_LONG_CONSTANT("VIR_DOMAIN_DISK_FILE",  
> DOMAIN_DISK_FILE, CONST_CS | CONST_PERSISTENT);
> - REGISTER_LONG_CONSTANT("VIR_DOMAIN_DISK_BLOCK", 
> DOMAIN_DISK_BLOCK, CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_DOMAIN_DISK_BLOCK", 
> DOMAIN_DISK_BLOCK, CONST_CS | CONST_PERSISTENT);

This looks like spurious change.

>   REGISTER_LONG_CONSTANT("VIR_DOMAIN_DISK_ACCESS_ALL",
> DOMAIN_DISK_ACCESS_ALL, CONST_CS | CONST_PERSISTENT);
>  
>   /* Domain metadata constants */
> @@ -1385,6 +1395,24 @@ PHP_MINIT_FUNCTION(libvirt)
>   REGISTER_LONG_CONSTANT("VIR_DOMAIN_AFFECT_LIVE",1, 
> CONST_CS | CONST_PERSISTENT);
>   REGISTER_LONG_CONSTANT("VIR_DOMAIN_AFFECT_CONFIG",  2, 
> CONST_CS | CONST_PERSISTENT);
>  
> + REGISTER_LONG_CONSTANT("VIR_DOMAIN_STATS_STATE",
> VIR_DOMAIN_STATS_STATE, CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_DOMAIN_STATS_CPU_TOTAL",
> VIR_DOMAIN_STATS_CPU_TOTAL, CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_DOMAIN_STATS_BALLOON",  
> VIR_DOMAIN_STATS_BALLOON, CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_DOMAIN_STATS_VCPU", 
> VIR_DOMAIN_STATS_VCPU, CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_DOMAIN_STATS_INTERFACE",
> VIR_DOMAIN_STATS_INTERFACE, CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_DOMAIN_STATS_BLOCK",
> VIR_DOMAIN_STATS_BLOCK, CONST_CS | CONST_PERSISTENT);
> +
> + REGISTER_LONG_CONSTANT("VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE",  
> VIR_CONNECT_GET_ALL_DOMAINS_STATS_ACTIVE, CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_CONNECT_GET_ALL_DOMAINS_STATS_INACTIVE",
> VIR_CONNECT_GET_ALL_DOMAINS_STATS_INACTIVE, CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER",   
> VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER, CONST_CS | CONST_PERSISTENT);
> + REGISTER_LONG_CONSTANT("VI

Re: [libvirt] [sandbox PATCH 0/3] Disk support for libvirt-sandbox

[Cedric Bosdonnat]

On Wed, 2015-06-10 at 13:06 +0100, Daniel P. Berrange wrote:
> On Wed, Jun 10, 2015 at 01:40:07PM +0200, Eren Yagdiran wrote:
> > Hello,
> > 
> > These patches provide disk support for libvirt-sandbox.
> > Implemented '--disk' parameter will be useful when integrating Docker image 
> > support for libvirt-sandbox.
> > Currently, we only support qcow2 file format and fallback is set to RAW 
> > type.
> 
> Can you explain a bit more why you need to be able to expose a virtual
> disk to the sandbox. If the stuff running inside the sandbox is not
> privileged, it won't even have access to the device node inside it,
> nor be able to mount it. This is why the sandbox code focuses on
> mounting everything itself before handing off the main app to run.
> So I'm not really clear how this feature is going to be generally
> useful or usable.

Before filling the qcow2 images, we need to be able to format them...
would you then suggest to manually setup the nbd device, and run mkfs on
it? I was thinking about having that done through virt-sandbox...

--
Cedric

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Problem with setting up KVM guests to use HugePages

[Michal Privoznik]

On 10.06.2015 01:05, Vivi L wrote:
> Kashyap Chamarthy  redhat.com> writes:
> 
> 
>> You might want re-test by explicitly setting the 'page' element and
>> 'size' attribute? From my test, I had something like this:
>>
>> $ virsh dumpxml f21-vm | grep hugepages -B3 -A2 
>>   2000896
>>   200
>>   
>> 
>>   
>> 
>>   
>>   8
>>
>> I haven't tested this exhaustively, but some basic test notes here:
>>
>> https://kashyapc.fedorapeople.org/virt/test-hugepages-with-libvirt.txt
> 
> Current QEMU does not support setting  element. Could it be the 
> cause of my aforementioned problem?
> 
> unsupported configuration: huge pages per NUMA node are not supported 
> with this QEMU
> 

So this is explanation why the memory for you guest is not backed by
hugepages.

Can you please share the qemu command line that libvirt generated for
your guest?

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] util: Fix coverity warings in virProcessGetAffinity

[Peter Krempa]

On Thu, Jun 04, 2015 at 08:34:00 -0400, John Ferlan wrote:
> On 06/04/2015 08:15 AM, Peter Krempa wrote:
> > Refactor the code flow a bit more to clear coverity errors.
> > 
> > Store the cpu count in an intermediate variable and reuse it rather than
> > caluclating the index.
> > ---
> >  src/util/virprocess.c | 11 +++
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> > 

...

> 
> ^^^ Coverity still complains here
> 
> No real change since previous...
> 

So I went in and traced it a bit. cpu_set_t is basically the following
structure: (assuming sizeof(unsigned long int) == 8))

typedef struct
{
  unsigned long int __bits[1024 / 8 * 8];
} cpu_set_t;


The CPU_ALLOC() macro allocates an array of such structures. Since the
size of the __bits array is rounded nicely (128 bytes) the structure
itself does not add any padding and it's size is 128 bytes too the
structures are packed and thus the __bits fields of adjecent structures
basically form a longer array of unsigned long ints.

The CPU_ISSET_S macro then translates basically to this equivalent
function:

unsigned long int
CPU_ISSET_S(size_t __cpu,
size_t setsize,
cpu_set_t *cpusetp)
{
if (__cpu / 8 < setsize) {
unsigned long int subcpu =  ((const unsigned long int 
*)(mask->__bits))[__cpu / 64];
unsigned long int mask = (unsigned long int) 1 << (__cpu % 64);

return subcpu & mask;
} else {
return 0;
}
}

The macro expects that the array is packed nicely and treats it
basically as a large array of unsigned ints. The macro then selects one
of the members and masks out the required cpu bit.

So then compiling the following proof of concept:

#define _GNU_SOURCE
#include 

int main(void)
{
size_t ncpus = 1024 << 8;
size_t masklen = CPU_ALLOC_SIZE(ncpus);
cpu_set_t *mask = CPU_ALLOC(ncpus);

CPU_ZERO_S(masklen, mask);

for (size_t i = 0; i < ncpus; i++) {
if (CPU_ISSET_S(i, masklen, mask)) {
i = i;
}
}
CPU_FREE(mask);

return 0;
}

And running it in valgrind results in no error as expected:

$ valgrind ./a.out 
==95609== Memcheck, a memory error detector
==95609== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==95609== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==95609== Command: ./a.out
==95609== 
==95609== 
==95609== HEAP SUMMARY:
==95609== in use at exit: 0 bytes in 0 blocks
==95609==   total heap usage: 1 allocs, 1 frees, 32,768 bytes allocated
==95609== 
==95609== All heap blocks were freed -- no leaks are possible
==95609== 
==95609== For counts of detected and suppressed errors, rerun with: -v
==95609== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

The problem is that coverity thinks that the code overruns the __bits
array now that the code is simple enough to introspect which is
technically true. Previously the same situation would happen but the
loop that would resize the array incrementally probably was not
introspected properly and thus did not produce a warning.

So there are basically three options:
1) Silence the coverity warning
2) File a bug against libc or something to fix the macro
3) Reimplement CPU_ISSET_S internally. (Which I don't think will be
possible since cpu_set_t does not look public)

At any rate, there is no problem in libvirt's usage as it conforms to
the example in the man page.

As of this particular patch. It does not fix the coverity warning, but I
think the intention and code flow is more apparent once it's applied.

Peter


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 08/24] Pass domain object to private data formatter/parser

[Jiri Denemark]

So that they can format private data (e.g., disk private data) stored
elsewhere in the domain object.

Signed-off-by: Jiri Denemark 
---

Notes:
ACKed in version 2

Version 2, 3:
- no changes

 src/conf/domain_conf.c   |  4 ++--
 src/conf/domain_conf.h   |  6 --
 src/libxl/libxl_domain.c | 10 ++
 src/lxc/lxc_domain.c | 12 
 src/qemu/qemu_domain.c   | 10 ++
 5 files changed, 26 insertions(+), 16 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 433183f..350640f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15974,7 +15974,7 @@ virDomainObjParseXML(xmlDocPtr xml,
 VIR_FREE(nodes);
 
 if (xmlopt->privateData.parse &&
-((xmlopt->privateData.parse)(ctxt, obj->privateData)) < 0)
+xmlopt->privateData.parse(ctxt, obj) < 0)
 goto error;
 
 return obj;
@@ -21863,7 +21863,7 @@ virDomainObjFormat(virDomainXMLOptionPtr xmlopt,
 }
 
 if (xmlopt->privateData.format &&
-((xmlopt->privateData.format)(&buf, obj->privateData)) < 0)
+xmlopt->privateData.format(&buf, obj) < 0)
 goto error;
 
 if (virDomainDefFormatInternal(obj->def, flags, &buf) < 0)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ac29ce5..44ecd4a 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2357,8 +2357,10 @@ typedef virDomainXMLOption *virDomainXMLOptionPtr;
 typedef void *(*virDomainXMLPrivateDataAllocFunc)(void);
 typedef void (*virDomainXMLPrivateDataFreeFunc)(void *);
 typedef virObjectPtr (*virDomainXMLPrivateDataNewFunc)(void);
-typedef int (*virDomainXMLPrivateDataFormatFunc)(virBufferPtr, void *);
-typedef int (*virDomainXMLPrivateDataParseFunc)(xmlXPathContextPtr, void *);
+typedef int (*virDomainXMLPrivateDataFormatFunc)(virBufferPtr,
+ virDomainObjPtr);
+typedef int (*virDomainXMLPrivateDataParseFunc)(xmlXPathContextPtr,
+virDomainObjPtr);
 
 /* Called once after everything else has been parsed, for adjusting
  * overall domain defaults.  */
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index 0652270..ebd7964 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -223,9 +223,10 @@ libxlDomainObjPrivateFree(void *data)
 }
 
 static int
-libxlDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, void *data)
+libxlDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
+  virDomainObjPtr vm)
 {
-libxlDomainObjPrivatePtr priv = data;
+libxlDomainObjPrivatePtr priv = vm->privateData;
 
 priv->lockState = virXPathString("string(./lockstate)", ctxt);
 
@@ -233,9 +234,10 @@ libxlDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, 
void *data)
 }
 
 static int
-libxlDomainObjPrivateXMLFormat(virBufferPtr buf, void *data)
+libxlDomainObjPrivateXMLFormat(virBufferPtr buf,
+   virDomainObjPtr vm)
 {
-libxlDomainObjPrivatePtr priv = data;
+libxlDomainObjPrivatePtr priv = vm->privateData;
 
 if (priv->lockState)
 virBufferAsprintf(buf, "%s\n", priv->lockState);
diff --git a/src/lxc/lxc_domain.c b/src/lxc/lxc_domain.c
index c2180cb..70606f3 100644
--- a/src/lxc/lxc_domain.c
+++ b/src/lxc/lxc_domain.c
@@ -51,9 +51,11 @@ static void virLXCDomainObjPrivateFree(void *data)
 }
 
 
-static int virLXCDomainObjPrivateXMLFormat(virBufferPtr buf, void *data)
+static int
+virLXCDomainObjPrivateXMLFormat(virBufferPtr buf,
+virDomainObjPtr vm)
 {
-virLXCDomainObjPrivatePtr priv = data;
+virLXCDomainObjPrivatePtr priv = vm->privateData;
 
 virBufferAsprintf(buf, "\n",
   (unsigned long long)priv->initpid);
@@ -61,9 +63,11 @@ static int virLXCDomainObjPrivateXMLFormat(virBufferPtr buf, 
void *data)
 return 0;
 }
 
-static int virLXCDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, void *data)
+static int
+virLXCDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
+   virDomainObjPtr vm)
 {
-virLXCDomainObjPrivatePtr priv = data;
+virLXCDomainObjPrivatePtr priv = vm->privateData;
 unsigned long long thepid;
 
 if (virXPathULongLong("string(./init[1]/@pid)", ctxt, &thepid) < 0) {
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0b5ebe1..24ff020 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -511,9 +511,10 @@ qemuDomainObjPrivateFree(void *data)
 
 
 static int
-qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data)
+qemuDomainObjPrivateXMLFormat(virBufferPtr buf,
+  virDomainObjPtr vm)
 {
-qemuDomainObjPrivatePtr priv = data;
+qemuDomainObjPrivatePtr priv = vm->privateData;
 const char *monitorpath;
 qemuDomainJob job;
 
@@ -600,9 +601,10 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data)
 }
 
 static int
-qemuDomainObjPrivateXMLParse(xmlXPathContextPtr 

[libvirt] [PATCH v3 02/24] qemu: Introduce qemuBlockJobUpdate

[Jiri Denemark]

The wrapper is useful for calling qemuBlockJobEventProcess with the
event details stored in disk's privateData, which is the most likely
usage of qemuBlockJobEventProcess.

Signed-off-by: Jiri Denemark 
---

Notes:
ACKed in version 1 and 2

Version 3:
- better flow in qemuBlockJobUpdate
- document qemuBlockJobUpdate

Version 2:
- no changes

ACKed in version 1 and 2

Version 3:
- better flow in qemuBlockJobUpdate

Version 2:
- no changes

 src/libvirt_private.syms |  2 ++
 src/qemu/qemu_blockjob.c | 47 +++
 src/qemu/qemu_blockjob.h |  3 +++
 3 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 62a4b4c..0c9fa06 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -265,6 +265,8 @@ virDomainDiskInsert;
 virDomainDiskInsertPreAlloced;
 virDomainDiskIoTypeFromString;
 virDomainDiskIoTypeToString;
+virDomainDiskMirrorStateTypeFromString;
+virDomainDiskMirrorStateTypeToString;
 virDomainDiskPathByName;
 virDomainDiskRemove;
 virDomainDiskRemoveByName;
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 098a43a..eb05cef 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -38,6 +38,37 @@
 
 VIR_LOG_INIT("qemu.qemu_blockjob");
 
+
+/**
+ * qemuBlockJobUpdate:
+ * @driver: qemu driver
+ * @vm: domain
+ * @disk: domain disk
+ *
+ * Update disk's mirror state in response to a block job event stored in
+ * blockJobStatus by qemuProcessHandleBlockJob event handler.
+ *
+ * Returns the block job event processed or -1 if there was no pending event.
+ */
+int
+qemuBlockJobUpdate(virQEMUDriverPtr driver,
+   virDomainObjPtr vm,
+   virDomainDiskDefPtr disk)
+{
+qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+int status = diskPriv->blockJobStatus;
+
+if (status != -1) {
+qemuBlockJobEventProcess(driver, vm, disk,
+ diskPriv->blockJobType,
+ diskPriv->blockJobStatus);
+diskPriv->blockJobStatus = -1;
+}
+
+return status;
+}
+
+
 /**
  * qemuBlockJobEventProcess:
  * @driver: qemu driver
@@ -49,8 +80,6 @@ VIR_LOG_INIT("qemu.qemu_blockjob");
  * Update disk's mirror state in response to a block job event
  * from QEMU. For mirror state's that must survive libvirt
  * restart, also update the domain's status XML.
- *
- * Returns 0 on success, -1 otherwise.
  */
 void
 qemuBlockJobEventProcess(virQEMUDriverPtr driver,
@@ -67,6 +96,12 @@ qemuBlockJobEventProcess(virQEMUDriverPtr driver,
 bool save = false;
 qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
 
+VIR_DEBUG("disk=%s, mirrorState=%s, type=%d, status=%d",
+  disk->dst,
+  NULLSTR(virDomainDiskMirrorStateTypeToString(disk->mirrorState)),
+  type,
+  status);
+
 /* Have to generate two variants of the event for old vs. new
  * client callbacks */
 if (type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT &&
@@ -218,9 +253,7 @@ qemuBlockJobSyncEnd(virQEMUDriverPtr driver,
 if (diskPriv->blockJobSync && diskPriv->blockJobStatus != -1) {
 if (ret_status)
 *ret_status = diskPriv->blockJobStatus;
-qemuBlockJobEventProcess(driver, vm, disk,
- diskPriv->blockJobType,
- diskPriv->blockJobStatus);
+qemuBlockJobUpdate(driver, vm, disk);
 diskPriv->blockJobStatus = -1;
 }
 diskPriv->blockJobSync = false;
@@ -300,9 +333,7 @@ qemuBlockJobSyncWaitWithTimeout(virQEMUDriverPtr driver,
 
 if (ret_status)
 *ret_status = diskPriv->blockJobStatus;
-qemuBlockJobEventProcess(driver, vm, disk,
- diskPriv->blockJobType,
- diskPriv->blockJobStatus);
+qemuBlockJobUpdate(driver, vm, disk);
 diskPriv->blockJobStatus = -1;
 
 return 0;
diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h
index ba372a2..81e893e 100644
--- a/src/qemu/qemu_blockjob.h
+++ b/src/qemu/qemu_blockjob.h
@@ -25,6 +25,9 @@
 # include "internal.h"
 # include "qemu_conf.h"
 
+int qemuBlockJobUpdate(virQEMUDriverPtr driver,
+   virDomainObjPtr vm,
+   virDomainDiskDefPtr disk);
 void qemuBlockJobEventProcess(virQEMUDriverPtr driver,
   virDomainObjPtr vm,
   virDomainDiskDefPtr disk,
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 10/24] qemu: Refactor qemuMonitorBlockJobInfo

[Jiri Denemark]

"query-block-jobs" QMP command returns all running block jobs at once,
while qemuMonitorBlockJobInfo would only report one. This is not very
nice in case we need to check several block jobs. This patch refactors
the monitor code to always parse all block jobs and store them in a
hash.

Signed-off-by: Jiri Denemark 
---

Notes:
ACKed in version 2

Version 3:
- no change

Version 2:
- new patch

 src/qemu/qemu_driver.c   | 45 +++-
 src/qemu/qemu_monitor.c  | 37 ++-
 src/qemu/qemu_monitor.h  | 17 ---
 src/qemu/qemu_monitor_json.c | 70 ++--
 src/qemu/qemu_monitor_json.h |  7 ++---
 5 files changed, 104 insertions(+), 72 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0214e6b..1556a9e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16178,7 +16178,7 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,
 {
 int ret = -1, rc;
 qemuDomainObjPrivatePtr priv = vm->privateData;
-virDomainBlockJobInfo info;
+qemuMonitorBlockJobInfo info;
 virStorageSourcePtr oldsrc = NULL;
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 
@@ -16192,7 +16192,7 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,
 /* Probe the status, if needed.  */
 if (!disk->mirrorState) {
 qemuDomainObjEnterMonitor(driver, vm);
-rc = qemuMonitorBlockJobInfo(priv->mon, device, &info, NULL);
+rc = qemuMonitorGetBlockJobInfo(priv->mon, disk->info.alias, &info);
 if (qemuDomainObjExitMonitor(driver, vm) < 0)
 goto cleanup;
 if (rc < 0)
@@ -16525,16 +16525,16 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
 
 
 static int
-qemuDomainGetBlockJobInfo(virDomainPtr dom, const char *path,
-   virDomainBlockJobInfoPtr info, unsigned int flags)
+qemuDomainGetBlockJobInfo(virDomainPtr dom,
+  const char *path,
+  virDomainBlockJobInfoPtr info,
+  unsigned int flags)
 {
 virQEMUDriverPtr driver = dom->conn->privateData;
 virDomainObjPtr vm;
-char *device = NULL;
-int idx;
 virDomainDiskDefPtr disk;
 int ret = -1;
-unsigned long long bandwidth;
+qemuMonitorBlockJobInfo rawInfo;
 
 virCheckFlags(VIR_DOMAIN_BLOCK_JOB_INFO_BANDWIDTH_BYTES, -1);
 
@@ -16557,31 +16557,34 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom, const 
char *path,
 if (qemuDomainSupportsBlockJobs(vm, NULL) < 0)
 goto endjob;
 
-if (!(device = qemuDiskPathToAlias(vm, path, &idx)))
+if (!(disk = virDomainDiskByName(vm->def, path, true)))
 goto endjob;
-disk = vm->def->disks[idx];
 
 qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorBlockJobInfo(qemuDomainGetMonitor(vm), device, info,
-  &bandwidth);
+ret = qemuMonitorGetBlockJobInfo(qemuDomainGetMonitor(vm),
+ disk->info.alias, &rawInfo);
 if (qemuDomainObjExitMonitor(driver, vm) < 0)
 ret = -1;
 if (ret < 0)
 goto endjob;
 
+info->cur = rawInfo.cur;
+info->end = rawInfo.end;
+
+info->type = rawInfo.type;
 if (info->type == VIR_DOMAIN_BLOCK_JOB_TYPE_COMMIT &&
 disk->mirrorJob == VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT)
 info->type = disk->mirrorJob;
-if (bandwidth) {
-if (!(flags & VIR_DOMAIN_BLOCK_JOB_INFO_BANDWIDTH_BYTES))
-bandwidth = VIR_DIV_UP(bandwidth, 1024 * 1024);
-info->bandwidth = bandwidth;
-if (info->bandwidth != bandwidth) {
-virReportError(VIR_ERR_OVERFLOW,
-   _("bandwidth %llu cannot be represented in result"),
-   bandwidth);
-goto endjob;
-}
+
+if (rawInfo.bandwidth &&
+!(flags & VIR_DOMAIN_BLOCK_JOB_INFO_BANDWIDTH_BYTES))
+rawInfo.bandwidth = VIR_DIV_UP(rawInfo.bandwidth, 1024 * 1024);
+info->bandwidth = rawInfo.bandwidth;
+if (info->bandwidth != rawInfo.bandwidth) {
+virReportError(VIR_ERR_OVERFLOW,
+   _("bandwidth %llu cannot be represented in result"),
+   rawInfo.bandwidth);
+goto endjob;
 }
 
 /* Snoop block copy operations, so future cancel operations can
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 33600f0..ae7ef28 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3207,17 +3207,40 @@ qemuMonitorBlockJobSetSpeed(qemuMonitorPtr mon,
 }
 
 
+virHashTablePtr
+qemuMonitorGetAllBlockJobInfo(qemuMonitorPtr mon)
+{
+QEMU_CHECK_MONITOR_JSON_NULL(mon);
+return qemuMonitorJSONGetAllBlockJobInfo(mon);
+}
+
+
+/**
+ * qemuMonitorGetBlockJobInfo:
+ * Parse Block Job information, and populate info for the named device.
+ * Return 1 if info available, 0 if device has no block job, and -1 on error.
+ */
 int
-qe

[libvirt] [PATCH v3 03/24] qemu: Properly report failed migration

[Jiri Denemark]

Because we are polling we may detect some errors after we asked QEMU for
migration status even though they occurred before. If this happens and
QEMU reports migration completed successfully, we would happily report
the migration succeeded even though we should have cancelled it because
of the other error.

In practise it is not a big issue now but it will become a much bigger
issue once the check for storage migration status is moved inside the
loop in qemuMigrationWaitForCompletion.

Signed-off-by: Jiri Denemark 
---

Notes:
ACKed in version 2

Version 3:
- no change

Version 2:
- already ACKed in version 1 :-)
- really do what commit message describes

 src/qemu/qemu_migration.c | 48 +++
 1 file changed, 23 insertions(+), 25 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 70400f3..8d01468 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2442,6 +2442,7 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver,
 qemuDomainJobInfoPtr jobInfo = priv->job.current;
 const char *job;
 int pauseReason;
+int ret = -1;
 
 switch (priv->job.asyncJob) {
 case QEMU_ASYNC_JOB_MIGRATION_OUT:
@@ -2459,12 +2460,12 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver,
 
 jobInfo->type = VIR_DOMAIN_JOB_UNBOUNDED;
 
-while (1) {
+while (jobInfo->type == VIR_DOMAIN_JOB_UNBOUNDED) {
 /* Poll every 50ms for progress & to allow cancellation */
 struct timespec ts = { .tv_sec = 0, .tv_nsec = 50 * 1000 * 1000ull };
 
 if (qemuMigrationUpdateJobStatus(driver, vm, job, asyncJob) == -1)
-break;
+goto error;
 
 /* cancel migration if disk I/O error is emitted while migrating */
 if (abort_on_error &&
@@ -2472,40 +2473,37 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver,
 pauseReason == VIR_DOMAIN_PAUSED_IOERROR) {
 virReportError(VIR_ERR_OPERATION_FAILED,
_("%s: %s"), job, _("failed due to I/O error"));
-break;
+goto error;
 }
 
 if (dconn && virConnectIsAlive(dconn) <= 0) {
 virReportError(VIR_ERR_OPERATION_FAILED, "%s",
_("Lost connection to destination host"));
-break;
+goto error;
 }
 
-if (jobInfo->type != VIR_DOMAIN_JOB_UNBOUNDED)
-break;
-
-virObjectUnlock(vm);
-
-nanosleep(&ts, NULL);
-
-virObjectLock(vm);
+if (jobInfo->type == VIR_DOMAIN_JOB_UNBOUNDED) {
+virObjectUnlock(vm);
+nanosleep(&ts, NULL);
+virObjectLock(vm);
+}
 }
 
-if (jobInfo->type == VIR_DOMAIN_JOB_COMPLETED) {
-qemuDomainJobInfoUpdateDowntime(jobInfo);
-VIR_FREE(priv->job.completed);
-if (VIR_ALLOC(priv->job.completed) == 0)
-*priv->job.completed = *jobInfo;
-return 0;
-} else if (jobInfo->type == VIR_DOMAIN_JOB_UNBOUNDED) {
-/* The migration was aborted by us rather than QEMU itself so let's
- * update the job type and notify the caller to send migrate_cancel.
- */
+qemuDomainJobInfoUpdateDowntime(jobInfo);
+VIR_FREE(priv->job.completed);
+if (VIR_ALLOC(priv->job.completed) == 0)
+*priv->job.completed = *jobInfo;
+return 0;
+
+ error:
+/* Check if the migration was aborted by us rather than QEMU itself. */
+if (jobInfo->type == VIR_DOMAIN_JOB_UNBOUNDED ||
+jobInfo->type == VIR_DOMAIN_JOB_COMPLETED) {
+if (jobInfo->type == VIR_DOMAIN_JOB_UNBOUNDED)
+ret = -2;
 jobInfo->type = VIR_DOMAIN_JOB_FAILED;
-return -2;
-} else {
-return -1;
 }
+return ret;
 }
 
 
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 04/24] qemu: Use domain condition for synchronous block jobs

[Jiri Denemark]

By switching block jobs to use domain conditions, we can drop some
pretty complicated code in NBD storage migration.

Signed-off-by: Jiri Denemark 
---

Notes:
Version 3:
- split into 3 patches

Version 2:
- slightly modified to use domain conditions

 po/POTFILES.in|   1 -
 src/qemu/qemu_blockjob.c  | 137 +++---
 src/qemu/qemu_blockjob.h  |  12 +---
 src/qemu/qemu_domain.c|  17 +-
 src/qemu/qemu_domain.h|   1 -
 src/qemu/qemu_driver.c|  24 
 src/qemu/qemu_migration.c | 112 +
 src/qemu/qemu_process.c   |  13 ++---
 8 files changed, 76 insertions(+), 241 deletions(-)

diff --git a/po/POTFILES.in b/po/POTFILES.in
index bb0f6e1..dd06ab3 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -112,7 +112,6 @@ src/parallels/parallels_utils.h
 src/parallels/parallels_storage.c
 src/phyp/phyp_driver.c
 src/qemu/qemu_agent.c
-src/qemu/qemu_blockjob.c
 src/qemu/qemu_capabilities.c
 src/qemu/qemu_cgroup.c
 src/qemu/qemu_command.c
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index eb05cef..3aa6118 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -214,19 +214,17 @@ qemuBlockJobEventProcess(virQEMUDriverPtr driver,
  *
  * During a synchronous block job, a block job event for @disk
  * will not be processed asynchronously. Instead, it will be
- * processed only when qemuBlockJobSyncWait* or
- * qemuBlockJobSyncEnd is called.
+ * processed only when qemuBlockJobUpdate or qemuBlockJobSyncEnd
+ * is called.
  */
 void
 qemuBlockJobSyncBegin(virDomainDiskDefPtr disk)
 {
 qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
 
-if (diskPriv->blockJobSync)
-VIR_WARN("Disk %s already has synchronous block job",
- disk->dst);
-
+VIR_DEBUG("disk=%s", disk->dst);
 diskPriv->blockJobSync = true;
+diskPriv->blockJobStatus = -1;
 }
 
 
@@ -235,135 +233,16 @@ qemuBlockJobSyncBegin(virDomainDiskDefPtr disk)
  * @driver: qemu driver
  * @vm: domain
  * @disk: domain disk
- * @ret_status: pointer to virConnectDomainEventBlockJobStatus
  *
  * End a synchronous block job for @disk. Any pending block job event
- * for the disk is processed, and its status is recorded in the
- * virConnectDomainEventBlockJobStatus field pointed to by
- * @ret_status.
+ * for the disk is processed.
  */
 void
 qemuBlockJobSyncEnd(virQEMUDriverPtr driver,
 virDomainObjPtr vm,
-virDomainDiskDefPtr disk,
-virConnectDomainEventBlockJobStatus *ret_status)
+virDomainDiskDefPtr disk)
 {
-qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
-
-if (diskPriv->blockJobSync && diskPriv->blockJobStatus != -1) {
-if (ret_status)
-*ret_status = diskPriv->blockJobStatus;
-qemuBlockJobUpdate(driver, vm, disk);
-diskPriv->blockJobStatus = -1;
-}
-diskPriv->blockJobSync = false;
-}
-
-
-/**
- * qemuBlockJobSyncWaitWithTimeout:
- * @driver: qemu driver
- * @vm: domain
- * @disk: domain disk
- * @timeout: timeout in milliseconds
- * @ret_status: pointer to virConnectDomainEventBlockJobStatus
- *
- * Wait up to @timeout milliseconds for a block job event for @disk.
- * If an event is received it is processed, and its status is recorded
- * in the virConnectDomainEventBlockJobStatus field pointed to by
- * @ret_status.
- *
- * If @timeout is not 0, @vm will be unlocked while waiting for the event.
- *
- * Returns 0 if an event was received or the timeout expired,
- *-1 otherwise.
- */
-int
-qemuBlockJobSyncWaitWithTimeout(virQEMUDriverPtr driver,
-virDomainObjPtr vm,
-virDomainDiskDefPtr disk,
-unsigned long long timeout,
-virConnectDomainEventBlockJobStatus 
*ret_status)
-{
-qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
-
-if (!diskPriv->blockJobSync) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("No current synchronous block job"));
-return -1;
-}
-
-while (diskPriv->blockJobSync && diskPriv->blockJobStatus == -1) {
-int r;
-
-if (!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("guest unexpectedly quit"));
-diskPriv->blockJobSync = false;
-return -1;
-}
-
-if (timeout == (unsigned long long)-1) {
-r = virCondWait(&diskPriv->blockJobSyncCond, &vm->parent.lock);
-} else if (timeout) {
-unsigned long long now;
-if (virTimeMillisNow(&now) < 0) {
-virReportSystemError(errno, "%s",
- _("Unable to get current time"));
-return -1;
-}
-r = virCond

[libvirt] [PATCH v3 05/24] qemu: Cancel storage migration in parallel

[Jiri Denemark]

Instead of cancelling disk mirrors sequentially, let's just call
block-job-cancel for all migrating disks and then wait until all
disappear.

In case we cancel disk mirrors at the end of successful migration we
also need to check all block jobs completed successfully. Otherwise we
have to abort the migration.

Signed-off-by: Jiri Denemark 
---

Notes:
Version 3:
- new patch (separated from "qemu: Use domain condition for
  synchronous block jobs")
- get rid of bool *failed parameter in qemuMigrationDriveMirrorCancelled

 src/qemu/qemu_migration.c | 196 +++---
 1 file changed, 134 insertions(+), 62 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 83d6c22..11504eb 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1769,76 +1769,122 @@ qemuMigrationDriveMirrorReady(virQEMUDriverPtr driver,
 }
 
 
-/**
- * qemuMigrationCancelOneDriveMirror:
- * @driver: qemu driver
- * @vm: domain
+/*
+ * If @check is true, the function will report an error and return a different
+ * code in case a block job fails. This way we can properly abort migration in
+ * case some block jobs failed once all memory has already been transferred.
  *
- * Cancel all drive-mirrors started by qemuMigrationDriveMirror.
- * Any pending block job events for the mirrored disks will be
- * processed.
- *
- * Returns 0 on success, -1 otherwise.
+ * Returns 1 if all mirrors are gone,
+ * 0 if some mirrors are still active,
+ * -1 some mirrors failed but some are still active,
+ * -2 all mirrors are gone but some of them failed.
  */
 static int
-qemuMigrationCancelOneDriveMirror(virQEMUDriverPtr driver,
+qemuMigrationDriveMirrorCancelled(virQEMUDriverPtr driver,
   virDomainObjPtr vm,
-  virDomainDiskDefPtr disk)
+  bool check)
 {
-qemuDomainObjPrivatePtr priv = vm->privateData;
-char *diskAlias = NULL;
-int ret = -1;
+size_t i;
+size_t active = 0;
+int status;
+bool failed = false;
 
-/* No need to cancel if mirror already aborted */
-if (disk->mirrorState == VIR_DOMAIN_DISK_MIRROR_STATE_ABORT) {
-ret = 0;
-} else {
-virConnectDomainEventBlockJobStatus status = -1;
+for (i = 0; i < vm->def->ndisks; i++) {
+virDomainDiskDefPtr disk = vm->def->disks[i];
+qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
 
-if (virAsprintf(&diskAlias, "%s%s",
-QEMU_DRIVE_HOST_PREFIX, disk->info.alias) < 0)
-goto cleanup;
+if (!diskPriv->migrating)
+continue;
 
-if (qemuDomainObjEnterMonitorAsync(driver, vm,
-   QEMU_ASYNC_JOB_MIGRATION_OUT) < 0)
-goto endjob;
-ret = qemuMonitorBlockJobCancel(priv->mon, diskAlias, true);
-if (qemuDomainObjExitMonitor(driver, vm) < 0)
-goto endjob;
-
-if (ret < 0) {
-virDomainBlockJobInfo info;
-
-/* block-job-cancel can fail if QEMU simultaneously
- * aborted the job; probe for it again to detect this */
-if (qemuMonitorBlockJobInfo(priv->mon, diskAlias,
-&info, NULL) == 0) {
-ret = 0;
-} else {
+status = qemuBlockJobUpdate(driver, vm, disk);
+switch (status) {
+case VIR_DOMAIN_BLOCK_JOB_FAILED:
+if (check) {
 virReportError(VIR_ERR_OPERATION_FAILED,
-   _("could not cancel migration of disk %s"),
+   _("migration of disk %s failed"),
disk->dst);
+failed = true;
 }
+/* fallthrough */
+case VIR_DOMAIN_BLOCK_JOB_CANCELED:
+case VIR_DOMAIN_BLOCK_JOB_COMPLETED:
+qemuBlockJobSyncEnd(driver, vm, disk);
+diskPriv->migrating = false;
+break;
 
-goto endjob;
+default:
+active++;
 }
+}
 
-/* Mirror may become ready before cancellation takes
- * effect; loop if we get that event first */
-while (1) {
-status = qemuBlockJobUpdate(driver, vm, disk);
-if (status != -1 && status != VIR_DOMAIN_BLOCK_JOB_READY)
-break;
-if ((ret = virDomainObjWait(vm)) < 0)
-goto endjob;
+if (failed) {
+if (active) {
+VIR_DEBUG("Some disk mirrors failed; still waiting for %zu "
+  "disk mirrors to finish", active);
+return -1;
+} else {
+VIR_DEBUG("All disk mirrors are gone; some of them failed");
+return -2;
+}
+} else {
+if (active) {
+VIR_DEBUG("Waiting for %zu disk mirrors to finish", acti

[libvirt] [PATCH v3 06/24] qemu: Abort migration early if disk mirror failed

[Jiri Denemark]

Abort migration as soon as we detect that some of the disk mirrors
failed. There's no sense in trying to finish memory migration first.

Signed-off-by: Jiri Denemark 
---

Notes:
Version 3:
- new patch (separated from "qemu: Use domain condition for
  synchronous block jobs")

 src/qemu/qemu_migration.c | 20 +---
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 11504eb..b11407e 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2498,7 +2498,8 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver,
virDomainObjPtr vm,
qemuDomainAsyncJob asyncJob,
virConnectPtr dconn,
-   bool abort_on_error)
+   bool abort_on_error,
+   bool storage)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
 qemuDomainJobInfoPtr jobInfo = priv->job.current;
@@ -2529,6 +2530,10 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver,
 if (qemuMigrationUpdateJobStatus(driver, vm, job, asyncJob) == -1)
 goto error;
 
+if (storage &&
+qemuMigrationDriveMirrorReady(driver, vm) < 0)
+break;
+
 /* cancel migration if disk I/O error is emitted while migrating */
 if (abort_on_error &&
 virDomainObjGetState(vm, &pauseReason) == VIR_DOMAIN_PAUSED &&
@@ -4146,20 +4151,12 @@ qemuMigrationRun(virQEMUDriverPtr driver,
 
 rc = qemuMigrationWaitForCompletion(driver, vm,
 QEMU_ASYNC_JOB_MIGRATION_OUT,
-dconn, abort_on_error);
+dconn, abort_on_error, !!mig->nbd);
 if (rc == -2)
 goto cancel;
 else if (rc == -1)
 goto cleanup;
 
-/* Confirm state of drive mirrors */
-if (mig->nbd) {
-if (qemuMigrationDriveMirrorReady(driver, vm) != 1) {
-ret = -1;
-goto cancel;
-}
-}
-
 /* When migration completed, QEMU will have paused the
  * CPUs for us, but unless we're using the JSON monitor
  * we won't have been notified of this, so might still
@@ -5637,7 +5634,8 @@ qemuMigrationToFile(virQEMUDriverPtr driver, 
virDomainObjPtr vm,
 if (rc < 0)
 goto cleanup;
 
-rc = qemuMigrationWaitForCompletion(driver, vm, asyncJob, NULL, false);
+rc = qemuMigrationWaitForCompletion(driver, vm, asyncJob,
+NULL, false, false);
 
 if (rc < 0) {
 if (rc == -2) {
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 11/24] qemu: Cancel disk mirrors after libvirtd restart

[Jiri Denemark]

When libvirtd is restarted during migration, we properly cancel the
ongoing migration (unless it managed to almost finished before the
restart). But if we were also migrating storage using NBD, we would
completely forget about the running disk mirrors.

Signed-off-by: Jiri Denemark 
---

Notes:
ACKed in version 2

Version 3:
- no change

Version 2:
- make use of qemuMonitorGetAllBlockJobInfo introduced by the previous patch
- undo qemuBlockJobSyncBegin in case of error

 src/qemu/qemu_domain.c| 45 -
 src/qemu/qemu_migration.c | 85 +++
 src/qemu/qemu_migration.h |  3 ++
 src/qemu/qemu_process.c   |  8 +
 4 files changed, 133 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 24ff020..68b6a95 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -578,7 +578,27 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf,
   qemuDomainAsyncJobPhaseToString(
 priv->job.asyncJob, priv->job.phase));
 }
-virBufferAddLit(buf, "/>\n");
+if (priv->job.asyncJob != QEMU_ASYNC_JOB_MIGRATION_OUT) {
+virBufferAddLit(buf, "/>\n");
+} else {
+size_t i;
+virDomainDiskDefPtr disk;
+qemuDomainDiskPrivatePtr diskPriv;
+
+virBufferAddLit(buf, ">\n");
+virBufferAdjustIndent(buf, 2);
+
+for (i = 0; i < vm->def->ndisks; i++) {
+disk = vm->def->disks[i];
+diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+virBufferAsprintf(buf, "\n",
+  disk->dst,
+  diskPriv->migrating ? "yes" : "no");
+}
+
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, "\n");
+}
 }
 priv->job.active = job;
 
@@ -736,6 +756,29 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
 }
 }
 
+if ((n = virXPathNodeSet("./job[1]/disk[@migrating='yes']",
+ ctxt, &nodes)) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("failed to parse list of disks marked for 
migration"));
+goto error;
+}
+if (n > 0) {
+if (priv->job.asyncJob != QEMU_ASYNC_JOB_MIGRATION_OUT) {
+VIR_WARN("Found disks marked for migration but we were not "
+ "migrating");
+n = 0;
+}
+for (i = 0; i < n; i++) {
+char *dst = virXMLPropString(nodes[i], "dev");
+virDomainDiskDefPtr disk;
+
+if (dst && (disk = virDomainDiskByName(vm->def, dst, false)))
+QEMU_DOMAIN_DISK_PRIVATE(disk)->migrating = true;
+VIR_FREE(dst);
+}
+}
+VIR_FREE(nodes);
+
 priv->fakeReboot = virXPathBoolean("boolean(./fakereboot)", ctxt) == 1;
 
 if ((n = virXPathNodeSet("./devices/device", ctxt, &nodes)) < 0) {
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 065a0f9..e0de09e 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2000,6 +2000,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver,
 char *hoststr = NULL;
 unsigned int mirror_flags = VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT;
 int rv;
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 
 VIR_DEBUG("Starting drive mirrors for domain %s", vm->def->name);
 
@@ -2049,6 +2050,11 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver,
 goto cleanup;
 }
 diskPriv->migrating = true;
+
+if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) {
+VIR_WARN("Failed to save status on vm %s", vm->def->name);
+goto cleanup;
+}
 }
 
 while ((rv = qemuMigrationDriveMirrorReady(driver, vm)) != 1) {
@@ -2076,6 +2082,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver,
 ret = 0;
 
  cleanup:
+virObjectUnref(cfg);
 VIR_FREE(diskAlias);
 VIR_FREE(nbd_dest);
 VIR_FREE(hoststr);
@@ -5698,6 +5705,84 @@ qemuMigrationToFile(virQEMUDriverPtr driver, 
virDomainObjPtr vm,
 return ret;
 }
 
+
+int
+qemuMigrationCancel(virQEMUDriverPtr driver,
+virDomainObjPtr vm)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+virHashTablePtr blockJobs = NULL;
+bool storage = false;
+size_t i;
+int ret = -1;
+
+VIR_DEBUG("Canceling unfinished outgoing migration of domain %s",
+  vm->def->name);
+
+for (i = 0; i < vm->def->ndisks; i++) {
+virDomainDiskDefPtr disk = vm->def->disks[i];
+if (QEMU_DOMAIN_DISK_PRIVATE(disk)->migrating) {
+qemuBlockJobSyncBegin(disk);
+storage = true;
+}
+}
+
+qemuDomainObjEnterMonitor(driver, vm);
+
+ignore_value(qemuMonitorMigrateCancel(priv->mon));
+

[libvirt] [PATCH v3 09/24] qemu: Make qemuMigrationCancelDriveMirror usable without async job

[Jiri Denemark]

We don't have an async job when reconnecting to existing domains after
libvirtd restart.

Signed-off-by: Jiri Denemark 
---

Notes:
ACKed in version 2

Version 2, 3:
- no changes

 src/qemu/qemu_migration.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index b11407e..065a0f9 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1847,7 +1847,8 @@ static int
 qemuMigrationCancelOneDriveMirror(virQEMUDriverPtr driver,
   virDomainObjPtr vm,
   virDomainDiskDefPtr disk,
-  bool failNoJob)
+  bool failNoJob,
+  qemuDomainAsyncJob asyncJob)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
 char *diskAlias = NULL;
@@ -1875,8 +1876,7 @@ qemuMigrationCancelOneDriveMirror(virQEMUDriverPtr driver,
 QEMU_DRIVE_HOST_PREFIX, disk->info.alias) < 0)
 return -1;
 
-if (qemuDomainObjEnterMonitorAsync(driver, vm,
-   QEMU_ASYNC_JOB_MIGRATION_OUT) < 0)
+if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
 goto cleanup;
 
 rv = qemuMonitorBlockJobCancel(priv->mon, diskAlias, true);
@@ -1907,7 +1907,8 @@ qemuMigrationCancelOneDriveMirror(virQEMUDriverPtr driver,
 static int
 qemuMigrationCancelDriveMirror(virQEMUDriverPtr driver,
virDomainObjPtr vm,
-   bool check)
+   bool check,
+   qemuDomainAsyncJob asyncJob)
 {
 virErrorPtr err = NULL;
 int ret = -1;
@@ -1924,7 +1925,8 @@ qemuMigrationCancelDriveMirror(virQEMUDriverPtr driver,
 if (!diskPriv->migrating)
 continue;
 
-rv = qemuMigrationCancelOneDriveMirror(driver, vm, disk, check);
+rv = qemuMigrationCancelOneDriveMirror(driver, vm, disk,
+   check, asyncJob);
 if (rv != 0) {
 if (rv < 0) {
 if (!err)
@@ -3609,7 +3611,8 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver,
 virErrorPtr orig_err = virSaveLastError();
 
 /* cancel any outstanding NBD jobs */
-qemuMigrationCancelDriveMirror(driver, vm, false);
+qemuMigrationCancelDriveMirror(driver, vm, false,
+   QEMU_ASYNC_JOB_MIGRATION_OUT);
 
 virSetError(orig_err);
 virFreeError(orig_err);
@@ -4180,7 +4183,8 @@ qemuMigrationRun(virQEMUDriverPtr driver,
 
 /* cancel any outstanding NBD jobs */
 if (mig && mig->nbd) {
-if (qemuMigrationCancelDriveMirror(driver, vm, ret == 0) < 0)
+if (qemuMigrationCancelDriveMirror(driver, vm, ret == 0,
+   QEMU_ASYNC_JOB_MIGRATION_OUT) < 0)
 ret = -1;
 }
 
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 17/24] qemu: Don't pass redundant job name around

[Jiri Denemark]

Instead of passing current job name to several functions which already
know what the current job is we can generate the name where we actually
need to use it.

Signed-off-by: Jiri Denemark 
---

Notes:
ACKed in version 2

Version 3:
- no change

Version 2:
- new patch

 src/qemu/qemu_migration.c | 54 ---
 1 file changed, 28 insertions(+), 26 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 7259c57..77a76a7 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2468,6 +2468,24 @@ qemuMigrationFetchJobStatus(virQEMUDriverPtr driver,
 }
 
 
+static const char *
+qemuMigrationJobName(virDomainObjPtr vm)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+
+switch (priv->job.asyncJob) {
+case QEMU_ASYNC_JOB_MIGRATION_OUT:
+return _("migration job");
+case QEMU_ASYNC_JOB_SAVE:
+return _("domain save job");
+case QEMU_ASYNC_JOB_DUMP:
+return _("domain core dump job");
+default:
+return _("job");
+}
+}
+
+
 static int
 qemuMigrationUpdateJobStatus(virQEMUDriverPtr driver,
  virDomainObjPtr vm,
@@ -2488,7 +2506,6 @@ qemuMigrationUpdateJobStatus(virQEMUDriverPtr driver,
 static int
 qemuMigrationCheckJobStatus(virQEMUDriverPtr driver,
 virDomainObjPtr vm,
-const char *job,
 qemuDomainAsyncJob asyncJob)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
@@ -2499,18 +2516,18 @@ qemuMigrationCheckJobStatus(virQEMUDriverPtr driver,
 
 switch (jobInfo->type) {
 case VIR_DOMAIN_JOB_NONE:
-virReportError(VIR_ERR_OPERATION_FAILED,
-   _("%s: %s"), job, _("is not active"));
+virReportError(VIR_ERR_OPERATION_FAILED, _("%s: %s"),
+   qemuMigrationJobName(vm), _("is not active"));
 return -1;
 
 case VIR_DOMAIN_JOB_FAILED:
-virReportError(VIR_ERR_OPERATION_FAILED,
-   _("%s: %s"), job, _("unexpectedly failed"));
+virReportError(VIR_ERR_OPERATION_FAILED, _("%s: %s"),
+   qemuMigrationJobName(vm), _("unexpectedly failed"));
 return -1;
 
 case VIR_DOMAIN_JOB_CANCELLED:
-virReportError(VIR_ERR_OPERATION_ABORTED,
-   _("%s: %s"), job, _("canceled by client"));
+virReportError(VIR_ERR_OPERATION_ABORTED, _("%s: %s"),
+   qemuMigrationJobName(vm), _("canceled by client"));
 return -1;
 
 case VIR_DOMAIN_JOB_BOUNDED:
@@ -2536,31 +2553,16 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver,
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
 qemuDomainJobInfoPtr jobInfo = priv->job.current;
-const char *job;
 int pauseReason;
 int ret = -1;
 
-switch (priv->job.asyncJob) {
-case QEMU_ASYNC_JOB_MIGRATION_OUT:
-job = _("migration job");
-break;
-case QEMU_ASYNC_JOB_SAVE:
-job = _("domain save job");
-break;
-case QEMU_ASYNC_JOB_DUMP:
-job = _("domain core dump job");
-break;
-default:
-job = _("job");
-}
-
 jobInfo->type = VIR_DOMAIN_JOB_UNBOUNDED;
 
 while (jobInfo->type == VIR_DOMAIN_JOB_UNBOUNDED) {
 /* Poll every 50ms for progress & to allow cancellation */
 struct timespec ts = { .tv_sec = 0, .tv_nsec = 50 * 1000 * 1000ull };
 
-if (qemuMigrationCheckJobStatus(driver, vm, job, asyncJob) < 0)
+if (qemuMigrationCheckJobStatus(driver, vm, asyncJob) < 0)
 goto error;
 
 if (storage &&
@@ -2571,8 +2573,8 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver,
 if (abort_on_error &&
 virDomainObjGetState(vm, &pauseReason) == VIR_DOMAIN_PAUSED &&
 pauseReason == VIR_DOMAIN_PAUSED_IOERROR) {
-virReportError(VIR_ERR_OPERATION_FAILED,
-   _("%s: %s"), job, _("failed due to I/O error"));
+virReportError(VIR_ERR_OPERATION_FAILED, _("%s: %s"),
+   qemuMigrationJobName(vm), _("failed due to I/O 
error"));
 goto error;
 }
 
@@ -4166,7 +4168,7 @@ qemuMigrationRun(virQEMUDriverPtr driver,
  * rather failed later on.  Check its status before waiting for a
  * connection from qemu which may never be initiated.
  */
-if (qemuMigrationCheckJobStatus(driver, vm, _("migration job"),
+if (qemuMigrationCheckJobStatus(driver, vm,
 QEMU_ASYNC_JOB_MIGRATION_OUT) < 0)
 goto cancel;
 
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 00/24] Add support for migration events

[Jiri Denemark]

QEMU will soon (patches are available on qemu-devel) get support for
migration events which will finally allow us to get rid of polling
query-migrate every 50ms. However, we first need to be able to wait for
all events related to migration (migration status changes, block job
events, async abort requests) at once. This series prepares the
infrastructure and uses it to switch all polling loops in migration code
to pthread_cond_wait.

https://bugzilla.redhat.com/show_bug.cgi?id=1212077

Version 3 (see individual patches for details):
- most of the series has been ACKed in v2
- "qemu: Use domain condition for synchronous block jobs" was split in 3
  patches for easier review
- minor changes requested in v2 review

Version 2 (see individual patches for details):
- rewritten using per-domain condition variable
- enahnced to fully support the migration events


Jiri Denemark (24):
  conf: Introduce per-domain condition variable
  qemu: Introduce qemuBlockJobUpdate
  qemu: Properly report failed migration
  qemu: Use domain condition for synchronous block jobs
  qemu: Cancel storage migration in parallel
  qemu: Abort migration early if disk mirror failed
  qemu: Don't mess with disk->mirrorState
  Pass domain object to private data formatter/parser
  qemu: Make qemuMigrationCancelDriveMirror usable without async job
  qemu: Refactor qemuMonitorBlockJobInfo
  qemu: Cancel disk mirrors after libvirtd restart
  qemu: Use domain condition for asyncAbort
  qemu_monitor: Wire up SPICE_MIGRATE_COMPLETED event
  qemu: Do not poll for spice migration status
  qemu: Refactor qemuDomainGetJob{Info,Stats}
  qemu: Refactor qemuMigrationUpdateJobStatus
  qemu: Don't pass redundant job name around
  qemu: Refactor qemuMigrationWaitForCompletion
  qemu_monitor: Wire up MIGRATION event
  qemuDomainGetJobStatsInternal: Support migration events
  qemu: Update migration state according to MIGRATION event
  qemu: Wait for migration events on domain condition
  qemu: cancel drive mirrors when p2p connection breaks
  DO NOT APPLY: qemu: Work around weird migration status changes

 po/POTFILES.in   |   1 -
 src/conf/domain_conf.c   |  51 ++-
 src/conf/domain_conf.h   |  12 +-
 src/libvirt_private.syms |   6 +
 src/libxl/libxl_domain.c |  10 +-
 src/lxc/lxc_domain.c |  12 +-
 src/qemu/qemu_blockjob.c | 185 +++
 src/qemu/qemu_blockjob.h |  15 +-
 src/qemu/qemu_capabilities.c |   3 +
 src/qemu/qemu_capabilities.h |   1 +
 src/qemu/qemu_domain.c   |  78 +++--
 src/qemu/qemu_domain.h   |   7 +-
 src/qemu/qemu_driver.c   | 201 +++-
 src/qemu/qemu_migration.c| 763 +--
 src/qemu/qemu_migration.h|   8 +
 src/qemu/qemu_monitor.c  |  73 -
 src/qemu/qemu_monitor.h  |  33 +-
 src/qemu/qemu_monitor_json.c | 152 -
 src/qemu/qemu_monitor_json.h |   7 +-
 src/qemu/qemu_process.c  |  92 +-
 tests/qemumonitorjsontest.c  |  40 ---
 21 files changed, 1057 insertions(+), 693 deletions(-)

-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 13/24] qemu_monitor: Wire up SPICE_MIGRATE_COMPLETED event

[Jiri Denemark]

Signed-off-by: Jiri Denemark 
---

Notes:
ACKed in version 2

Version 3:
- no change

Version 2:
- new patch

 src/qemu/qemu_monitor.c  | 12 
 src/qemu/qemu_monitor.h  |  6 ++
 src/qemu/qemu_monitor_json.c | 10 ++
 3 files changed, 28 insertions(+)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index ae7ef28..b7de846 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1479,6 +1479,18 @@ qemuMonitorEmitSerialChange(qemuMonitorPtr mon,
 
 
 int
+qemuMonitorEmitSpiceMigrated(qemuMonitorPtr mon)
+{
+int ret = -1;
+VIR_DEBUG("mon=%p", mon);
+
+QEMU_MONITOR_CALLBACK(mon, ret, domainSpiceMigrated, mon->vm);
+
+return ret;
+}
+
+
+int
 qemuMonitorSetCapabilities(qemuMonitorPtr mon)
 {
 QEMU_CHECK_MONITOR(mon);
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 96f47e8..a29c505 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -182,6 +182,10 @@ typedef int 
(*qemuMonitorDomainSerialChangeCallback)(qemuMonitorPtr mon,
  bool connected,
  void *opaque);
 
+typedef int (*qemuMonitorDomainSpiceMigratedCallback)(qemuMonitorPtr mon,
+  virDomainObjPtr vm,
+  void *opaque);
+
 typedef struct _qemuMonitorCallbacks qemuMonitorCallbacks;
 typedef qemuMonitorCallbacks *qemuMonitorCallbacksPtr;
 struct _qemuMonitorCallbacks {
@@ -209,6 +213,7 @@ struct _qemuMonitorCallbacks {
 qemuMonitorDomainDeviceDeletedCallback domainDeviceDeleted;
 qemuMonitorDomainNicRxFilterChangedCallback domainNicRxFilterChanged;
 qemuMonitorDomainSerialChangeCallback domainSerialChange;
+qemuMonitorDomainSpiceMigratedCallback domainSpiceMigrated;
 };
 
 char *qemuMonitorEscapeArg(const char *in);
@@ -307,6 +312,7 @@ int qemuMonitorEmitNicRxFilterChanged(qemuMonitorPtr mon,
 int qemuMonitorEmitSerialChange(qemuMonitorPtr mon,
 const char *devAlias,
 bool connected);
+int qemuMonitorEmitSpiceMigrated(qemuMonitorPtr mon);
 
 int qemuMonitorStartCPUs(qemuMonitorPtr mon,
  virConnectPtr conn);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 5b227cd..04ae339 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -83,6 +83,7 @@ static void qemuMonitorJSONHandleGuestPanic(qemuMonitorPtr 
mon, virJSONValuePtr
 static void qemuMonitorJSONHandleDeviceDeleted(qemuMonitorPtr mon, 
virJSONValuePtr data);
 static void qemuMonitorJSONHandleNicRxFilterChanged(qemuMonitorPtr mon, 
virJSONValuePtr data);
 static void qemuMonitorJSONHandleSerialChange(qemuMonitorPtr mon, 
virJSONValuePtr data);
+static void qemuMonitorJSONHandleSpiceMigrated(qemuMonitorPtr mon, 
virJSONValuePtr data);
 
 typedef struct {
 const char *type;
@@ -107,6 +108,7 @@ static qemuEventHandler eventHandlers[] = {
 { "SPICE_CONNECTED", qemuMonitorJSONHandleSPICEConnect, },
 { "SPICE_DISCONNECTED", qemuMonitorJSONHandleSPICEDisconnect, },
 { "SPICE_INITIALIZED", qemuMonitorJSONHandleSPICEInitialize, },
+{ "SPICE_MIGRATE_COMPLETED", qemuMonitorJSONHandleSpiceMigrated, },
 { "STOP", qemuMonitorJSONHandleStop, },
 { "SUSPEND", qemuMonitorJSONHandlePMSuspend, },
 { "SUSPEND_DISK", qemuMonitorJSONHandlePMSuspendDisk, },
@@ -914,6 +916,14 @@ qemuMonitorJSONHandleSerialChange(qemuMonitorPtr mon,
 }
 
 
+static void
+qemuMonitorJSONHandleSpiceMigrated(qemuMonitorPtr mon,
+   virJSONValuePtr data ATTRIBUTE_UNUSED)
+{
+qemuMonitorEmitSpiceMigrated(mon);
+}
+
+
 int
 qemuMonitorJSONHumanCommandWithFd(qemuMonitorPtr mon,
   const char *cmd_str,
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 23/24] qemu: cancel drive mirrors when p2p connection breaks

[Jiri Denemark]

When a connection to the destination host during a p2p migration drops,
we know we will have to cancel the migration; it doesn't make sense to
waste resources by trying to finish the migration. We already do so
after sending "migrate" command to QEMU and we should do it while
waiting for drive mirrors to become ready too.

Signed-off-by: Jiri Denemark 
---

Notes:
ACKed in version 2

Version 3:
- rebased on top of modified qemuMigrationDriveMirrorCancelled

Version 2:
- new patch

 src/qemu/qemu_migration.c | 29 +++--
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 60c75f3..c53d2ea 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1908,7 +1908,8 @@ static int
 qemuMigrationCancelDriveMirror(virQEMUDriverPtr driver,
virDomainObjPtr vm,
bool check,
-   qemuDomainAsyncJob asyncJob)
+   qemuDomainAsyncJob asyncJob,
+   virConnectPtr dconn)
 {
 virErrorPtr err = NULL;
 int ret = -1;
@@ -1939,6 +1940,13 @@ qemuMigrationCancelDriveMirror(virQEMUDriverPtr driver,
 }
 
 while ((rv = qemuMigrationDriveMirrorCancelled(driver, vm, check)) != 1) {
+if (check && !failed &&
+dconn && virConnectIsAlive(dconn) <= 0) {
+virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+   _("Lost connection to destination host"));
+failed = true;
+}
+
 if (rv < 0) {
 failed = true;
 if (rv == -2)
@@ -1989,7 +1997,8 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver,
  qemuMigrationCookiePtr mig,
  const char *host,
  unsigned long speed,
- unsigned int *migrate_flags)
+ unsigned int *migrate_flags,
+ virConnectPtr dconn)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
 int ret = -1;
@@ -2069,6 +2078,12 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver,
 goto cleanup;
 }
 
+if (dconn && virConnectIsAlive(dconn) <= 0) {
+virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+   _("Lost connection to destination host"));
+goto cleanup;
+}
+
 if (virDomainObjWait(vm) < 0)
 goto cleanup;
 }
@@ -3689,7 +3704,7 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver,
 
 /* cancel any outstanding NBD jobs */
 qemuMigrationCancelDriveMirror(driver, vm, false,
-   QEMU_ASYNC_JOB_MIGRATION_OUT);
+   QEMU_ASYNC_JOB_MIGRATION_OUT, NULL);
 
 virSetError(orig_err);
 virFreeError(orig_err);
@@ -4106,7 +4121,8 @@ qemuMigrationRun(virQEMUDriverPtr driver,
 if (qemuMigrationDriveMirror(driver, vm, mig,
  spec->dest.host.name,
  migrate_speed,
- &migrate_flags) < 0) {
+ &migrate_flags,
+ dconn) < 0) {
 goto cleanup;
 }
 } else {
@@ -4265,7 +4281,8 @@ qemuMigrationRun(virQEMUDriverPtr driver,
 /* cancel any outstanding NBD jobs */
 if (mig && mig->nbd) {
 if (qemuMigrationCancelDriveMirror(driver, vm, ret == 0,
-   QEMU_ASYNC_JOB_MIGRATION_OUT) < 0)
+   QEMU_ASYNC_JOB_MIGRATION_OUT,
+   dconn) < 0)
 ret = -1;
 }
 
@@ -5853,7 +5870,7 @@ qemuMigrationCancel(virQEMUDriverPtr driver,
 }
 
 if (qemuMigrationCancelDriveMirror(driver, vm, false,
-   QEMU_ASYNC_JOB_NONE) < 0)
+   QEMU_ASYNC_JOB_NONE, NULL) < 0)
 goto endsyncjob;
 
 ret = 0;
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 20/24] qemuDomainGetJobStatsInternal: Support migration events

[Jiri Denemark]

When QEMU supports migration events the qemuDomainJobInfo structure will
no longer be updated with migration statistics. We have to enter a job
and explicitly ask QEMU every time virDomainGetJob{Info,Stats} is
called.

Signed-off-by: Jiri Denemark 
---

Notes:
ACKed in version 2

Version 3:
- moved before "qemu: Update migration state according to MIGRATION
  event"

Version 2:
- new patch

 src/qemu/qemu_driver.c | 27 +++
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 17c8c85..7c5d685 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13091,15 +13091,27 @@ qemuConnectBaselineCPU(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 
 
 static int
-qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
+qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver,
   virDomainObjPtr vm,
   bool completed,
   qemuDomainJobInfoPtr jobInfo)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
 qemuDomainJobInfoPtr info;
+bool fetch = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT);
 int ret = -1;
 
+if (completed)
+fetch = false;
+
+/* Do not ask QEMU if migration is not even running yet  */
+if (!priv->job.current || !priv->job.current->status.status)
+fetch = false;
+
+if (fetch &&
+qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
+return -1;
+
 if (!completed &&
 !virDomainObjIsActive(vm)) {
 virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -13120,12 +13132,19 @@ qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver 
ATTRIBUTE_UNUSED,
 *jobInfo = *info;
 
 if (jobInfo->type == VIR_DOMAIN_JOB_BOUNDED ||
-jobInfo->type == VIR_DOMAIN_JOB_UNBOUNDED)
-ret = qemuDomainJobInfoUpdateTime(jobInfo);
-else
+jobInfo->type == VIR_DOMAIN_JOB_UNBOUNDED) {
+if (fetch)
+ret = qemuMigrationFetchJobStatus(driver, vm, QEMU_ASYNC_JOB_NONE,
+  jobInfo);
+else
+ret = qemuDomainJobInfoUpdateTime(jobInfo);
+} else {
 ret = 0;
+}
 
  cleanup:
+if (fetch)
+qemuDomainObjEndJob(driver, vm);
 return ret;
 }
 
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 18/24] qemu: Refactor qemuMigrationWaitForCompletion

[Jiri Denemark]

Checking status of all part of migration and aborting it when something
failed is a complex thing which makes the waiting loop hard to read.
This patch moves all the checks into a separate function similarly to
what was done for drive mirror loops.

Signed-off-by: Jiri Denemark 
---

Notes:
ACKed in version 2

Version 3:
- no change

Version 2:
- new patch

 src/qemu/qemu_migration.c | 106 +-
 1 file changed, 66 insertions(+), 40 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 77a76a7..d9f1a59 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2540,6 +2540,63 @@ qemuMigrationCheckJobStatus(virQEMUDriverPtr driver,
 }
 
 
+/**
+ * Returns 1 if migration completed successfully,
+ * 0 if the domain is still being migrated,
+ * -1 migration failed,
+ * -2 something else failed, we need to cancel migration.
+ */
+static int
+qemuMigrationCompleted(virQEMUDriverPtr driver,
+   virDomainObjPtr vm,
+   qemuDomainAsyncJob asyncJob,
+   virConnectPtr dconn,
+   bool abort_on_error,
+   bool storage)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+qemuDomainJobInfoPtr jobInfo = priv->job.current;
+int pauseReason;
+
+if (qemuMigrationCheckJobStatus(driver, vm, asyncJob) < 0)
+goto error;
+
+if (storage && qemuMigrationDriveMirrorReady(driver, vm) < 0)
+goto error;
+
+if (abort_on_error &&
+virDomainObjGetState(vm, &pauseReason) == VIR_DOMAIN_PAUSED &&
+pauseReason == VIR_DOMAIN_PAUSED_IOERROR) {
+virReportError(VIR_ERR_OPERATION_FAILED, _("%s: %s"),
+   qemuMigrationJobName(vm), _("failed due to I/O error"));
+goto error;
+}
+
+if (dconn && virConnectIsAlive(dconn) <= 0) {
+virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+   _("Lost connection to destination host"));
+goto error;
+}
+
+if (jobInfo->type == VIR_DOMAIN_JOB_COMPLETED)
+return 1;
+else
+return 0;
+
+ error:
+if (jobInfo->type == VIR_DOMAIN_JOB_UNBOUNDED) {
+/* The migration was aborted by us rather than QEMU itself. */
+jobInfo->type = VIR_DOMAIN_JOB_FAILED;
+return -2;
+} else if (jobInfo->type == VIR_DOMAIN_JOB_COMPLETED) {
+jobInfo->type = VIR_DOMAIN_JOB_FAILED;
+return -1;
+} else {
+return -1;
+}
+}
+
+
 /* Returns 0 on success, -2 when migration needs to be cancelled, or -1 when
  * QEMU reports failed migration.
  */
@@ -2553,59 +2610,28 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver,
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
 qemuDomainJobInfoPtr jobInfo = priv->job.current;
-int pauseReason;
-int ret = -1;
+int rv;
 
 jobInfo->type = VIR_DOMAIN_JOB_UNBOUNDED;
-
-while (jobInfo->type == VIR_DOMAIN_JOB_UNBOUNDED) {
+while ((rv = qemuMigrationCompleted(driver, vm, asyncJob, dconn,
+abort_on_error, storage)) != 1) {
 /* Poll every 50ms for progress & to allow cancellation */
 struct timespec ts = { .tv_sec = 0, .tv_nsec = 50 * 1000 * 1000ull };
 
-if (qemuMigrationCheckJobStatus(driver, vm, asyncJob) < 0)
-goto error;
+if (rv < 0)
+return rv;
 
-if (storage &&
-qemuMigrationDriveMirrorReady(driver, vm) < 0)
-break;
-
-/* cancel migration if disk I/O error is emitted while migrating */
-if (abort_on_error &&
-virDomainObjGetState(vm, &pauseReason) == VIR_DOMAIN_PAUSED &&
-pauseReason == VIR_DOMAIN_PAUSED_IOERROR) {
-virReportError(VIR_ERR_OPERATION_FAILED, _("%s: %s"),
-   qemuMigrationJobName(vm), _("failed due to I/O 
error"));
-goto error;
-}
-
-if (dconn && virConnectIsAlive(dconn) <= 0) {
-virReportError(VIR_ERR_OPERATION_FAILED, "%s",
-   _("Lost connection to destination host"));
-goto error;
-}
-
-if (jobInfo->type == VIR_DOMAIN_JOB_UNBOUNDED) {
-virObjectUnlock(vm);
-nanosleep(&ts, NULL);
-virObjectLock(vm);
-}
+virObjectUnlock(vm);
+nanosleep(&ts, NULL);
+virObjectLock(vm);
 }
 
 qemuDomainJobInfoUpdateDowntime(jobInfo);
 VIR_FREE(priv->job.completed);
 if (VIR_ALLOC(priv->job.completed) == 0)
 *priv->job.completed = *jobInfo;
-return 0;
 
- error:
-/* Check if the migration was aborted by us rather than QEMU itself. */
-if (jobInfo->type == VIR_DOMAIN_JOB_UNBOUNDED ||
-jobInfo->type == VIR_DOMAIN_JOB_COMPLETED) {
-if (jobInfo->type == VIR_DOMAIN_JOB_UNBOUNDED)
-ret = -2;
-jobInf

[libvirt] [PATCH v3 16/24] qemu: Refactor qemuMigrationUpdateJobStatus

[Jiri Denemark]

Once we start waiting for migration events instead of polling
query-migrate, priv->job.current will not be regularly updated anymore
because we will get the current status directly from the events. Thus
virDomainGetJob{Info,Stats} will have to query QEMU, but they can't just
blindly update priv->job.current structure. This patch introduces
qemuMigrationFetchJobStatus which just fills in a caller supplied
structure and makes qemuMigrationUpdateJobStatus a tiny wrapper around
it.

Signed-off-by: Jiri Denemark 
---

Notes:
ACKed in version 2

Version 3:
- no change

Version 2:
- new patch

 src/qemu/qemu_migration.c | 133 ++
 src/qemu/qemu_migration.h |   5 ++
 2 files changed, 93 insertions(+), 45 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index d5a9dea..7259c57 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2416,67 +2416,110 @@ qemuMigrationWaitForSpice(virDomainObjPtr vm)
 return 0;
 }
 
-static int
-qemuMigrationUpdateJobStatus(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- const char *job,
- qemuDomainAsyncJob asyncJob)
+
+static void
+qemuMigrationUpdateJobType(qemuDomainJobInfoPtr jobInfo)
 {
-qemuDomainObjPrivatePtr priv = vm->privateData;
-qemuMonitorMigrationStatus status;
-qemuDomainJobInfoPtr jobInfo;
-int ret;
-
-memset(&status, 0, sizeof(status));
-
-ret = qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob);
-if (ret < 0) {
-/* Guest already exited or waiting for the job timed out; nothing
- * further to update. */
-return ret;
-}
-ret = qemuMonitorGetMigrationStatus(priv->mon, &status);
-
-if (qemuDomainObjExitMonitor(driver, vm) < 0)
-return -1;
-
-if (ret < 0 ||
-qemuDomainJobInfoUpdateTime(priv->job.current) < 0)
-return -1;
-
-ret = -1;
-jobInfo = priv->job.current;
-switch (status.status) {
+switch (jobInfo->status.status) {
 case QEMU_MONITOR_MIGRATION_STATUS_COMPLETED:
 jobInfo->type = VIR_DOMAIN_JOB_COMPLETED;
-/* fall through */
-case QEMU_MONITOR_MIGRATION_STATUS_SETUP:
-case QEMU_MONITOR_MIGRATION_STATUS_ACTIVE:
-case QEMU_MONITOR_MIGRATION_STATUS_CANCELLING:
-ret = 0;
 break;
 
 case QEMU_MONITOR_MIGRATION_STATUS_INACTIVE:
 jobInfo->type = VIR_DOMAIN_JOB_NONE;
-virReportError(VIR_ERR_OPERATION_FAILED,
-   _("%s: %s"), job, _("is not active"));
 break;
 
 case QEMU_MONITOR_MIGRATION_STATUS_ERROR:
 jobInfo->type = VIR_DOMAIN_JOB_FAILED;
-virReportError(VIR_ERR_OPERATION_FAILED,
-   _("%s: %s"), job, _("unexpectedly failed"));
 break;
 
 case QEMU_MONITOR_MIGRATION_STATUS_CANCELLED:
 jobInfo->type = VIR_DOMAIN_JOB_CANCELLED;
+break;
+
+case QEMU_MONITOR_MIGRATION_STATUS_SETUP:
+case QEMU_MONITOR_MIGRATION_STATUS_ACTIVE:
+case QEMU_MONITOR_MIGRATION_STATUS_CANCELLING:
+break;
+}
+}
+
+
+int
+qemuMigrationFetchJobStatus(virQEMUDriverPtr driver,
+virDomainObjPtr vm,
+qemuDomainAsyncJob asyncJob,
+qemuDomainJobInfoPtr jobInfo)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+int rv;
+
+if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+return -1;
+
+memset(&jobInfo->status, 0, sizeof(jobInfo->status));
+rv = qemuMonitorGetMigrationStatus(priv->mon, &jobInfo->status);
+
+if (qemuDomainObjExitMonitor(driver, vm) < 0 || rv < 0)
+return -1;
+
+qemuMigrationUpdateJobType(jobInfo);
+return qemuDomainJobInfoUpdateTime(jobInfo);
+}
+
+
+static int
+qemuMigrationUpdateJobStatus(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ qemuDomainAsyncJob asyncJob)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+qemuDomainJobInfoPtr jobInfo = priv->job.current;
+qemuDomainJobInfo newInfo = *jobInfo;
+
+if (qemuMigrationFetchJobStatus(driver, vm, asyncJob, &newInfo) < 0)
+return -1;
+
+*jobInfo = newInfo;
+return 0;
+}
+
+
+static int
+qemuMigrationCheckJobStatus(virQEMUDriverPtr driver,
+virDomainObjPtr vm,
+const char *job,
+qemuDomainAsyncJob asyncJob)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+qemuDomainJobInfoPtr jobInfo = priv->job.current;
+
+if (qemuMigrationUpdateJobStatus(driver, vm, asyncJob) < 0)
+return -1;
+
+switch (jobInfo->type) {
+case VIR_DOMAIN_JOB_NONE:
+virReportError(VIR_ERR_OPERATION_FAILED,
+   _("%s: %s"), job, _("is not active"));
+return -1;
+
+case VIR_DOMAIN_JOB_FAILED:

[libvirt] [PATCH v3 14/24] qemu: Do not poll for spice migration status

[Jiri Denemark]

QEMU_CAPS_SEAMLESS_MIGRATION capability says QEMU supports
SPICE_MIGRATE_COMPLETED event. Thus we can just drop all code which
polls query-spice and replace it with waiting for the event.

Signed-off-by: Jiri Denemark 
---

Notes:
ACKed in version 2

Version 3:
- no change

Version 2:
- new patch

 src/qemu/qemu_domain.c   |  1 +
 src/qemu/qemu_domain.h   |  1 +
 src/qemu/qemu_migration.c| 38 ++
 src/qemu/qemu_monitor.c  | 10 -
 src/qemu/qemu_monitor.h  |  2 --
 src/qemu/qemu_monitor_json.c | 49 
 src/qemu/qemu_process.c  | 28 +
 tests/qemumonitorjsontest.c  | 40 
 8 files changed, 41 insertions(+), 128 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 25fa8d3..c9bdf6b 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -170,6 +170,7 @@ qemuDomainObjResetAsyncJob(qemuDomainObjPrivatePtr priv)
 job->mask = QEMU_JOB_DEFAULT_MASK;
 job->dump_memory_only = false;
 job->abortJob = false;
+job->spiceMigrated = false;
 VIR_FREE(job->current);
 }
 
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index a3c5015..54e1e7b 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -136,6 +136,7 @@ struct qemuDomainJobObj {
 qemuDomainJobInfoPtr current;   /* async job progress data */
 qemuDomainJobInfoPtr completed; /* statistics data of a recently 
completed job */
 bool abortJob;  /* abort of the job requested */
+bool spiceMigrated; /* spice migration completed */
 };
 
 typedef void (*qemuDomainCleanupCallback)(virQEMUDriverPtr driver,
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index d82a5ba..d5a9dea 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2390,45 +2390,29 @@ qemuMigrationSetPinAll(virQEMUDriverPtr driver,
 }
 
 static int
-qemuMigrationWaitForSpice(virQEMUDriverPtr driver,
-  virDomainObjPtr vm)
+qemuMigrationWaitForSpice(virDomainObjPtr vm)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
 bool wait_for_spice = false;
-bool spice_migrated = false;
 size_t i = 0;
-int rc;
 
-if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SEAMLESS_MIGRATION)) {
-for (i = 0; i < vm->def->ngraphics; i++) {
-if (vm->def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
-wait_for_spice = true;
-break;
-}
+if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SEAMLESS_MIGRATION))
+return 0;
+
+for (i = 0; i < vm->def->ngraphics; i++) {
+if (vm->def->graphics[i]->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
+wait_for_spice = true;
+break;
 }
 }
 
 if (!wait_for_spice)
 return 0;
 
-while (!spice_migrated) {
-/* Poll every 50ms for progress & to allow cancellation */
-struct timespec ts = { .tv_sec = 0, .tv_nsec = 50 * 1000 * 1000ull };
-
-if (qemuDomainObjEnterMonitorAsync(driver, vm,
-   QEMU_ASYNC_JOB_MIGRATION_OUT) < 0)
+while (!priv->job.spiceMigrated && !priv->job.abortJob) {
+if (virDomainObjWait(vm) < 0)
 return -1;
-
-rc = qemuMonitorGetSpiceMigrationStatus(priv->mon, &spice_migrated);
-if (qemuDomainObjExitMonitor(driver, vm) < 0)
-return -1;
-if (rc < 0)
-return -1;
-virObjectUnlock(vm);
-nanosleep(&ts, NULL);
-virObjectLock(vm);
 }
-
 return 0;
 }
 
@@ -3602,7 +3586,7 @@ qemuMigrationConfirmPhase(virQEMUDriverPtr driver,
 if (retcode == 0) {
 /* If guest uses SPICE and supports seamless migration we have to hold
  * up domain shutdown until SPICE server transfers its data */
-qemuMigrationWaitForSpice(driver, vm);
+qemuMigrationWaitForSpice(vm);
 
 qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_MIGRATED,
 VIR_QEMU_PROCESS_STOP_MIGRATED);
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index b7de846..94b0007 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2117,16 +2117,6 @@ qemuMonitorGetMigrationStatus(qemuMonitorPtr mon,
 
 
 int
-qemuMonitorGetSpiceMigrationStatus(qemuMonitorPtr mon,
-   bool *spice_migrated)
-{
-QEMU_CHECK_MONITOR_JSON(mon);
-
-return qemuMonitorJSONGetSpiceMigrationStatus(mon, spice_migrated);
-}
-
-
-int
 qemuMonitorMigrateToFd(qemuMonitorPtr mon,
unsigned int flags,
int fd)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index a29c505..1afc344 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -498,8 +498,6 @@ struct _qemuMonitorMigration

[libvirt] [libvirt-php][PATCH] libvirt_connect_get_all_domain_stats: Add forgotten breaks

[Michal Privoznik]

Unfortunately, during review I haven't spotted the obvious. There's
missing break in each switch() case which will definitely produce a
segmentation fault as it causes strdup() to be called on non-string
types too. Sorry for the noise.

Signed-off-by: Michal Privoznik 
---

Pushed under trivial rule.

 src/libvirt-php.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index eb42994..698ab0f 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -2467,18 +2467,25 @@ PHP_FUNCTION(libvirt_connect_get_all_domain_stats)
switch (params.type) {
case VIR_TYPED_PARAM_INT:
add_assoc_long(arr2, params.field, 
params.value.i);
+   break;
case VIR_TYPED_PARAM_UINT:
add_assoc_long(arr2, params.field, 
params.value.ui);
+   break;
case VIR_TYPED_PARAM_LLONG:
add_assoc_long(arr2, params.field, 
params.value.l);
+   break;
case VIR_TYPED_PARAM_ULLONG:
add_assoc_long(arr2, params.field, 
params.value.ul);
+   break;
case VIR_TYPED_PARAM_DOUBLE:
add_assoc_double(arr2, params.field, 
params.value.d);
+   break;
case VIR_TYPED_PARAM_BOOLEAN:
add_assoc_bool(arr2, params.field, 
params.value.b);
+   break;
case VIR_TYPED_PARAM_STRING:
add_assoc_string_ex(arr2, params.field, 
strlen(params.field)+1, params.value.s, strlen(params.value.s)+1);
+   break;
}
}
name = virDomainGetName(retstats[i]->dom);
-- 
2.3.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [RFC] libvirt support multi-thread compression for live migration

[Jiri Denemark]

On Fri, Jun 05, 2015 at 08:22:31 +0800, Qiao,Liyong wrote:
> 
> 
> On 2015年06月04日 20:09, Jiri Denemark wrote:
> > On Tue, Jun 02, 2015 at 11:02:27 -0600, Eric Blake wrote:
> >> On 06/02/2015 07:56 AM, Qiao, Liyong wrote:
> >>> Hi Eric
> >>> Thanks for replying the mail, replied in lines.
> >>>
>  +virdomainMigrateGetParameters(virDomainPtr domain,
>  +  int *level,
>  +  int *threads,
>  +  int *dthreads,
>  +  int flags)
>  +
> >>> I'd rather we used virTypedParameters, to make it easier to use the same 
> >>> API to pass ALL future possible tuning parameters, rather than just 
> >>> hard-coding to only the parameters of this one feature.
> >>>
> >>> Okay ,that sound good, but if virTypedParameters can be passed to 
> >>> qemu_driver such as qemu_monitor_json.c qemu_monitor_text.c ?
> >> [Your quoting style makes it very hard to distinguish original text from
> >> added text.  Please consider changing your outgoing mail process to
> >> ensure that you add another level of quoting to all previous text so
> >> that your added text is the only unquoted text.  Also, configure your
> >> mailer to wrap long lines.]
> >>
> >> Use existing API for a guide - for example, virDomainSetBlockIoTune
> >> takes virTypedParamters, as well as defines a specific set of parameters
> >> that it will understand.
> > And do we actually need to changed these migration parameters on the fly
> > when migration is already running. I can imagine we would need to do so
> > for some parameters but multithreaded compression sounds like something
> > that needs to be configured when migration starts and there's nothing to
> > be tuned on the fly. If this is the case, I think we should just add
> > these new parameters to virDomainMigrate3 instead of requiring another
> > API to be called to actually configure multithreaded compression.
> >
> > The separate API makes it a bit harder to repeat migration (which
> > previously failed) with a different parameters (e.g., without
> > multithreaded compression).
> hi jirka
> thanks for your advice, that make sense for me.
> 
> if I understanding you correctly,
> then we need only do this
> 
> virsh migrate --live --compressed --compress-level 1--compress-threads 8 
> --decompress-threads 2
> 
> and all compressed parameters (compress-level ,compress-threads, 
> decompress-threads)  only
> be specified when we do call virDomainMigrate3 instead of expose to user 
> by 'virsh migrate-setparameters' ?

Right, although there still seems to be an open question whether we can
reuse --compressed or we need to come up with another option so that we
can independently turn on XBZRLE and multithreaded compression.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] question about virConnectGetAllDomainStats and virTypedParameter

[Michal Privoznik]

On 10.06.2015 08:52, Vasiliy Tolstov wrote:
> I'm try to implement virConnectGetAllDomainStats for php binding api,
> but have one issue with VIR_TYPED_PARAM_STRING:
> 
> code part:
> retval = virConnectGetAllDomainStats(conn->conn, stats,
> &retstats, flags);
> for (i=0; i < retval; i++) {
> zval *arr2;
> ALLOC_INIT_ZVAL(arr2);
> array_init(arr2);
> for (j = 0; j < retstats[i]->nparams; j++) {
> params = retstats[i]->params[j];
> switch (params.type) {
> case VIR_TYPED_PARAM_INT:
> add_assoc_long(arr2, params.field,
> params.value.i);
> case VIR_TYPED_PARAM_UINT:
> add_assoc_long(arr2, params.field,
> params.value.ui);
> case VIR_TYPED_PARAM_LLONG:
> add_assoc_long(arr2, params.field,
> params.value.l);
> case VIR_TYPED_PARAM_ULLONG:
> add_assoc_long(arr2, params.field,
> params.value.ul);
> case VIR_TYPED_PARAM_DOUBLE:
> add_assoc_double(arr2, params.field,
> params.value.d);
> case VIR_TYPED_PARAM_BOOLEAN:
> add_assoc_bool(arr2, params.field,
> params.value.b);
> case VIR_TYPED_PARAM_STRING:
> add_assoc_string_ex(arr2,
> params.field, strlen(params.field)+1, strdup(params.value.s),
> strlen(params.value.s)+1); // SEGFAULT HAPPENING
> }
> }
> 
> gdb shows:
> return_value_used=) at libvirt-php.c:2505
> arr2 = 0x77fd72b8
> conn = 
> zconn = 0x77fd7140
> retval = 
> flags = 
> stats = 
> name = 
> i = 
> j = 
> params = {field = "state.state", '\000' ,
> type = 1, value = {i = 5, ui = 5, l = 5, ul = 5, d =
> 2.4703282292062327e-323, b = 5 '\005',
> s = 0x5 }}
> retstats = 0x101d870
> 
> What i'm doing wrong?
> 

The switch() items needs to end with break; Otherwise add_assoc_*() will
be called that not correspond to the type. As in your example - the type
is INT, and you are seeing the error in strdup().

Unfortunately, my mind was too slow when reviewing your patch, so I've
pushed it without spotting it. I'm pushing the obvious fix right now.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 02/24] qemu: Introduce qemuBlockJobUpdate

[Peter Krempa]

On Wed, Jun 10, 2015 at 15:42:36 +0200, Jiri Denemark wrote:
> The wrapper is useful for calling qemuBlockJobEventProcess with the
> event details stored in disk's privateData, which is the most likely
> usage of qemuBlockJobEventProcess.
> 
> Signed-off-by: Jiri Denemark 
> ---
> 
> Notes:
> ACKed in version 1 and 2
> 
> Version 3:
> - better flow in qemuBlockJobUpdate
> - document qemuBlockJobUpdate
> 
> Version 2:
> - no changes
> 
> ACKed in version 1 and 2
> 
> Version 3:
> - better flow in qemuBlockJobUpdate
> 
> Version 2:
> - no changes
> 
>  src/libvirt_private.syms |  2 ++
>  src/qemu/qemu_blockjob.c | 47 +++
>  src/qemu/qemu_blockjob.h |  3 +++
>  3 files changed, 44 insertions(+), 8 deletions(-)
> 

...

> diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
> index 098a43a..eb05cef 100644
> --- a/src/qemu/qemu_blockjob.c
> +++ b/src/qemu/qemu_blockjob.c
> @@ -38,6 +38,37 @@
>  
>  VIR_LOG_INIT("qemu.qemu_blockjob");
>  
> +
> +/**
> + * qemuBlockJobUpdate:
> + * @driver: qemu driver
> + * @vm: domain
> + * @disk: domain disk
> + *
> + * Update disk's mirror state in response to a block job event stored in
> + * blockJobStatus by qemuProcessHandleBlockJob event handler.
> + *
> + * Returns the block job event processed or -1 if there was no pending event.
> + */
> +int
> +qemuBlockJobUpdate(virQEMUDriverPtr driver,
> +   virDomainObjPtr vm,
> +   virDomainDiskDefPtr disk)
> +{
> +qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
> +int status = diskPriv->blockJobStatus;
> +
> +if (status != -1) {
> +qemuBlockJobEventProcess(driver, vm, disk,
> + diskPriv->blockJobType,
> + diskPriv->blockJobStatus);
> +diskPriv->blockJobStatus = -1;
> +}
> +
> +return status;
> +}

Looks much better!

> +
> +
>  /**
>   * qemuBlockJobEventProcess:
>   * @driver: qemu driver

ACK,

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 v3 00/24] Add support for migration events

[John Ferlan]

On 06/10/2015 09:42 AM, Jiri Denemark wrote:
> QEMU will soon (patches are available on qemu-devel) get support for
> migration events which will finally allow us to get rid of polling
> query-migrate every 50ms. However, we first need to be able to wait for
> all events related to migration (migration status changes, block job
> events, async abort requests) at once. This series prepares the
> infrastructure and uses it to switch all polling loops in migration code
> to pthread_cond_wait.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1212077
> 
> Version 3 (see individual patches for details):
> - most of the series has been ACKed in v2
> - "qemu: Use domain condition for synchronous block jobs" was split in 3
>   patches for easier review
> - minor changes requested in v2 review
> 
> Version 2 (see individual patches for details):
> - rewritten using per-domain condition variable
> - enahnced to fully support the migration events
> 
> 
> Jiri Denemark (24):
>   conf: Introduce per-domain condition variable
>   qemu: Introduce qemuBlockJobUpdate
>   qemu: Properly report failed migration
>   qemu: Use domain condition for synchronous block jobs
>   qemu: Cancel storage migration in parallel
>   qemu: Abort migration early if disk mirror failed
>   qemu: Don't mess with disk->mirrorState
>   Pass domain object to private data formatter/parser
>   qemu: Make qemuMigrationCancelDriveMirror usable without async job
>   qemu: Refactor qemuMonitorBlockJobInfo
>   qemu: Cancel disk mirrors after libvirtd restart
>   qemu: Use domain condition for asyncAbort
>   qemu_monitor: Wire up SPICE_MIGRATE_COMPLETED event
>   qemu: Do not poll for spice migration status
>   qemu: Refactor qemuDomainGetJob{Info,Stats}
>   qemu: Refactor qemuMigrationUpdateJobStatus
>   qemu: Don't pass redundant job name around
>   qemu: Refactor qemuMigrationWaitForCompletion
>   qemu_monitor: Wire up MIGRATION event
>   qemuDomainGetJobStatsInternal: Support migration events
>   qemu: Update migration state according to MIGRATION event
>   qemu: Wait for migration events on domain condition
>   qemu: cancel drive mirrors when p2p connection breaks
>   DO NOT APPLY: qemu: Work around weird migration status changes
> 
>  po/POTFILES.in   |   1 -
>  src/conf/domain_conf.c   |  51 ++-
>  src/conf/domain_conf.h   |  12 +-
>  src/libvirt_private.syms |   6 +
>  src/libxl/libxl_domain.c |  10 +-
>  src/lxc/lxc_domain.c |  12 +-
>  src/qemu/qemu_blockjob.c | 185 +++
>  src/qemu/qemu_blockjob.h |  15 +-
>  src/qemu/qemu_capabilities.c |   3 +
>  src/qemu/qemu_capabilities.h |   1 +
>  src/qemu/qemu_domain.c   |  78 +++--
>  src/qemu/qemu_domain.h   |   7 +-
>  src/qemu/qemu_driver.c   | 201 +++-
>  src/qemu/qemu_migration.c| 763 
> +--
>  src/qemu/qemu_migration.h|   8 +
>  src/qemu/qemu_monitor.c  |  73 -
>  src/qemu/qemu_monitor.h  |  33 +-
>  src/qemu/qemu_monitor_json.c | 152 -
>  src/qemu/qemu_monitor_json.h |   7 +-
>  src/qemu/qemu_process.c  |  92 +-
>  tests/qemumonitorjsontest.c  |  40 ---
>  21 files changed, 1057 insertions(+), 693 deletions(-)
> 

Just ran this through my Coverity checker - only one issue

RESOURCE_LEAK in qemuMigrationRun:

4235if (qemuMigrationCheckJobStatus(driver, vm,
4236QEMU_ASYNC_JOB_MIGRATION_OUT) < 
0)

(4) Event if_end:   End of if statement

4237goto cancel;
4238

(5) Event open_fn:  Returning handle opened by "accept".
(6) Event var_assign:   Assigning: "fd" = handle returned from 
"accept(spec->dest.unix_socket.sock, __SOCKADDR_ARG({ .__sockaddr__ = NULL}), 
NULL)".
(7) Event cond_false:   Condition "(fd = accept(spec->dest.unix_socket.sock, 
__SOCKADDR_ARG({ .__sockaddr__ = NULL}), NULL)) < 0", taking false branch
Also see events:[leaked_handle]

4239while ((fd = accept(spec->dest.unix_socket.sock, NULL, NULL)) < 
0) {
4240if (errno == EAGAIN || errno == EINTR)
4241continue;
...
4252rc = qemuMigrationWaitForCompletion(driver, vm,
4253QEMU_ASYNC_JOB_MIGRATION_OUT,
4254dconn, abort_on_error, 
!!mig->nbd);

(13) Event cond_true:   Condition "rc == -2", taking true branch

4255if (rc == -2)

(14) Event goto:Jumping to label "cancel"

4256goto cancel;
4257else if (rc == -1)

...

4288

(28) Event cond_false:  Condition "spec->fwdType != MIGRATION_FWD_DIRECT", 
taking false branch

4289if (spec->fwdType != MIGRATION_FWD_DIRECT) {
4290if (iothread && qemuMigrationStopTunnel(iothread, ret < 0) < 0)
4291ret = -1;
4292VIR_FORCE_CLOSE(fd);

(29) Event if_end:  End of if statement

4293}
4294

...

4322}
4323

(38) Even

Re: [libvirt] [sandbox PATCH 0/3] Disk support for libvirt-sandbox

[Daniel P. Berrange]

On Wed, Jun 10, 2015 at 03:13:15PM +0200, Cedric Bosdonnat wrote:
> On Wed, 2015-06-10 at 13:06 +0100, Daniel P. Berrange wrote:
> > On Wed, Jun 10, 2015 at 01:40:07PM +0200, Eren Yagdiran wrote:
> > > Hello,
> > > 
> > > These patches provide disk support for libvirt-sandbox.
> > > Implemented '--disk' parameter will be useful when integrating Docker 
> > > image support for libvirt-sandbox.
> > > Currently, we only support qcow2 file format and fallback is set to RAW 
> > > type.
> > 
> > Can you explain a bit more why you need to be able to expose a virtual
> > disk to the sandbox. If the stuff running inside the sandbox is not
> > privileged, it won't even have access to the device node inside it,
> > nor be able to mount it. This is why the sandbox code focuses on
> > mounting everything itself before handing off the main app to run.
> > So I'm not really clear how this feature is going to be generally
> > useful or usable.
> 
> Before filling the qcow2 images, we need to be able to format them...
> would you then suggest to manually setup the nbd device, and run mkfs on
> it? I was thinking about having that done through virt-sandbox...

Ah ha, ok, that makes some sense now. So this isn't really for the purpose
of executing docker images - its for the earlier step where we actually
turn the downloaded docker images into something we can run. This stage
of the process would be running privileged, so it makes sense it can use
a block device.

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 v3 07/24] qemu: Don't mess with disk->mirrorState

[Jiri Denemark]

This patch reverts commit 76c61cdca20c106960af033e5d0f5da70177af0f.

VIR_DOMAIN_DISK_MIRROR_STATE_ABORT says we asked for a block job to be
aborted rather than saying it was aborted. Let's just use
VIR_DOMAIN_DISK_MIRROR_STATE_NONE consistently whenever a block job
finishes since no caller depends on VIR_DOMAIN_DISK_MIRROR_STATE_ABORT
(anymore) to check whether a block job failed or it was cancelled.

Signed-off-by: Jiri Denemark 
---

Notes:
ACKed in version 2

Version 2, 3:
- no changes

 src/qemu/qemu_blockjob.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 3aa6118..8849850 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -174,8 +174,7 @@ qemuBlockJobEventProcess(virQEMUDriverPtr driver,
 case VIR_DOMAIN_BLOCK_JOB_CANCELED:
 virStorageSourceFree(disk->mirror);
 disk->mirror = NULL;
-disk->mirrorState = status == VIR_DOMAIN_BLOCK_JOB_FAILED ?
-VIR_DOMAIN_DISK_MIRROR_STATE_ABORT : 
VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
+disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
 disk->mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_UNKNOWN;
 save = true;
 diskPriv->blockjob = false;
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 12/24] qemu: Use domain condition for asyncAbort

[Jiri Denemark]

To avoid polling for asyncAbort flag changes.

Signed-off-by: Jiri Denemark 
---

Notes:
ACKed in version 2

Version 3:
- no change

Version 2:
- rewritten using domain condition

 src/qemu/qemu_domain.c|  5 +++--
 src/qemu/qemu_domain.h|  2 +-
 src/qemu/qemu_migration.c | 11 ---
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 68b6a95..25fa8d3 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -169,7 +169,7 @@ qemuDomainObjResetAsyncJob(qemuDomainObjPrivatePtr priv)
 job->phase = 0;
 job->mask = QEMU_JOB_DEFAULT_MASK;
 job->dump_memory_only = false;
-job->asyncAbort = false;
+job->abortJob = false;
 VIR_FREE(job->current);
 }
 
@@ -1652,7 +1652,8 @@ qemuDomainObjAbortAsyncJob(virDomainObjPtr obj)
   qemuDomainAsyncJobTypeToString(priv->job.asyncJob),
   obj, obj->def->name);
 
-priv->job.asyncAbort = true;
+priv->job.abortJob = true;
+virDomainObjBroadcast(obj);
 }
 
 /*
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 9003c9b..a3c5015 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -135,7 +135,7 @@ struct qemuDomainJobObj {
 bool dump_memory_only;  /* use dump-guest-memory to do dump */
 qemuDomainJobInfoPtr current;   /* async job progress data */
 qemuDomainJobInfoPtr completed; /* statistics data of a recently 
completed job */
-bool asyncAbort;/* abort of async job requested */
+bool abortJob;  /* abort of the job requested */
 };
 
 typedef void (*qemuDomainCleanupCallback)(virQEMUDriverPtr driver,
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index e0de09e..d82a5ba 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2058,12 +2058,10 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver,
 }
 
 while ((rv = qemuMigrationDriveMirrorReady(driver, vm)) != 1) {
-unsigned long long now;
-
 if (rv < 0)
 goto cleanup;
 
-if (priv->job.asyncAbort) {
+if (priv->job.abortJob) {
 priv->job.current->type = VIR_DOMAIN_JOB_CANCELLED;
 virReportError(VIR_ERR_OPERATION_ABORTED, _("%s: %s"),
qemuDomainAsyncJobTypeToString(priv->job.asyncJob),
@@ -2071,8 +2069,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver,
 goto cleanup;
 }
 
-if (virTimeMillisNow(&now) < 0 ||
-virDomainObjWaitUntil(vm, now + 500) < 0)
+if (virDomainObjWait(vm) < 0)
 goto cleanup;
 }
 
@@ -4069,10 +4066,10 @@ qemuMigrationRun(virQEMUDriverPtr driver,
QEMU_ASYNC_JOB_MIGRATION_OUT) < 0)
 goto cleanup;
 
-if (priv->job.asyncAbort) {
+if (priv->job.abortJob) {
 /* explicitly do this *after* we entered the monitor,
  * as this is a critical section so we are guaranteed
- * priv->job.asyncAbort will not change */
+ * priv->job.abortJob will not change */
 ignore_value(qemuDomainObjExitMonitor(driver, vm));
 priv->job.current->type = VIR_DOMAIN_JOB_CANCELLED;
 virReportError(VIR_ERR_OPERATION_ABORTED, _("%s: %s"),
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 15/24] qemu: Refactor qemuDomainGetJob{Info, Stats}

[Jiri Denemark]

Move common parts of qemuDomainGetJobInfo and qemuDomainGetJobStats into
a separate API (qemuDomainGetJobStatsInternal).

Signed-off-by: Jiri Denemark 
---

Notes:
ACKed in version 2

Version 3:
- no change

Version 2:
- new patch

 src/qemu/qemu_driver.c | 113 ++---
 1 file changed, 61 insertions(+), 52 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1556a9e..17c8c85 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -13090,42 +13090,72 @@ qemuConnectBaselineCPU(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 }
 
 
-static int qemuDomainGetJobInfo(virDomainPtr dom,
-virDomainJobInfoPtr info)
+static int
+qemuDomainGetJobStatsInternal(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
+  virDomainObjPtr vm,
+  bool completed,
+  qemuDomainJobInfoPtr jobInfo)
 {
+qemuDomainObjPrivatePtr priv = vm->privateData;
+qemuDomainJobInfoPtr info;
+int ret = -1;
+
+if (!completed &&
+!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("domain is not running"));
+goto cleanup;
+}
+
+if (completed)
+info = priv->job.completed;
+else
+info = priv->job.current;
+
+if (!info) {
+jobInfo->type = VIR_DOMAIN_JOB_NONE;
+ret = 0;
+goto cleanup;
+}
+*jobInfo = *info;
+
+if (jobInfo->type == VIR_DOMAIN_JOB_BOUNDED ||
+jobInfo->type == VIR_DOMAIN_JOB_UNBOUNDED)
+ret = qemuDomainJobInfoUpdateTime(jobInfo);
+else
+ret = 0;
+
+ cleanup:
+return ret;
+}
+
+
+static int
+qemuDomainGetJobInfo(virDomainPtr dom,
+ virDomainJobInfoPtr info)
+{
+virQEMUDriverPtr driver = dom->conn->privateData;
+qemuDomainJobInfo jobInfo;
 virDomainObjPtr vm;
 int ret = -1;
-qemuDomainObjPrivatePtr priv;
 
 if (!(vm = qemuDomObjFromDomain(dom)))
 goto cleanup;
 
-priv = vm->privateData;
-
 if (virDomainGetJobInfoEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
 
-if (virDomainObjIsActive(vm)) {
-if (priv->job.current) {
-/* Refresh elapsed time again just to ensure it
- * is fully updated. This is primarily for benefit
- * of incoming migration which we don't currently
- * monitor actively in the background thread
- */
-if (qemuDomainJobInfoUpdateTime(priv->job.current) < 0 ||
-qemuDomainJobInfoToInfo(priv->job.current, info) < 0)
-goto cleanup;
-} else {
-memset(info, 0, sizeof(*info));
-info->type = VIR_DOMAIN_JOB_NONE;
-}
-} else {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
+if (qemuDomainGetJobStatsInternal(driver, vm, false, &jobInfo) < 0)
+goto cleanup;
+
+if (jobInfo.type == VIR_DOMAIN_JOB_NONE) {
+memset(info, 0, sizeof(*info));
+info->type = VIR_DOMAIN_JOB_NONE;
+ret = 0;
 goto cleanup;
 }
 
-ret = 0;
+ret = qemuDomainJobInfoToInfo(&jobInfo, info);
 
  cleanup:
 virDomainObjEndAPI(&vm);
@@ -13140,9 +13170,11 @@ qemuDomainGetJobStats(virDomainPtr dom,
   int *nparams,
   unsigned int flags)
 {
+virQEMUDriverPtr driver = dom->conn->privateData;
 virDomainObjPtr vm;
 qemuDomainObjPrivatePtr priv;
-qemuDomainJobInfoPtr jobInfo;
+qemuDomainJobInfo jobInfo;
+bool completed = !!(flags & VIR_DOMAIN_JOB_STATS_COMPLETED);
 int ret = -1;
 
 virCheckFlags(VIR_DOMAIN_JOB_STATS_COMPLETED, -1);
@@ -13150,24 +13182,14 @@ qemuDomainGetJobStats(virDomainPtr dom,
 if (!(vm = qemuDomObjFromDomain(dom)))
 goto cleanup;
 
-priv = vm->privateData;
-
 if (virDomainGetJobStatsEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;
 
-if (!(flags & VIR_DOMAIN_JOB_STATS_COMPLETED) &&
-!virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("domain is not running"));
+priv = vm->privateData;
+if (qemuDomainGetJobStatsInternal(driver, vm, completed, &jobInfo) < 0)
 goto cleanup;
-}
 
-if (flags & VIR_DOMAIN_JOB_STATS_COMPLETED)
-jobInfo = priv->job.completed;
-else
-jobInfo = priv->job.current;
-
-if (!jobInfo) {
+if (jobInfo.type == VIR_DOMAIN_JOB_NONE) {
 *type = VIR_DOMAIN_JOB_NONE;
 *params = NULL;
 *nparams = 0;
@@ -13175,24 +13197,11 @@ qemuDomainGetJobStats(virDomainPtr dom,
 goto cleanup;
 }
 
-/* Refresh elapsed time again just to ensure it
- * is fully updated. This is primarily for benefit
- * of incoming migration which we don't currently
- * mon

[libvirt] [PATCH v3 24/24] DO NOT APPLY: qemu: Work around weird migration status changes

[Jiri Denemark]

When cancelling migration we can see the following conversation on QMP
monitor:

{"execute":"migrate_cancel","id":"libvirt-33"}
{"timestamp": {"seconds": 1432899178, "microseconds": 844907}, "event":
"MIGRATION", "data": {"status": "cancelling"}}
{"return": {}, "id": "libvirt-33"}
{"timestamp": {"seconds": 1432899178, "microseconds": 845625}, "event":
"MIGRATION", "data": {"status": "failed"}}
{"timestamp": {"seconds": 1432899178, "microseconds": 846432}, "event":
"MIGRATION", "data": {"status": "cancelled"}}

That is, migration status first changes to "failed" just to change to
the correct "cancelled" state in a few moments later. However, this is
enough to let libvirt report migration failed for unknown reason instead
of having been cancelled by a user.

This should really be fixed in QEMU but I'm not sure how easy it is.
However, it's pretty easy for us to work around it.

Signed-off-by: Jiri Denemark 
---

Notes:
Version 3:
- mark as "DO NOT APPLY" -- Juan will fix the bug in QEMU in the
  next version of his migration event series

Version 2:
- new patch

 src/qemu/qemu_process.c | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 93c0844..dd43657 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1518,6 +1518,7 @@ qemuProcessHandleMigrationStatus(qemuMonitorPtr mon 
ATTRIBUTE_UNUSED,
  void *opaque ATTRIBUTE_UNUSED)
 {
 qemuDomainObjPrivatePtr priv;
+qemuDomainJobInfoPtr jobInfo;
 
 virObjectLock(vm);
 
@@ -1532,7 +1533,15 @@ qemuProcessHandleMigrationStatus(qemuMonitorPtr mon 
ATTRIBUTE_UNUSED,
 goto cleanup;
 }
 
-priv->job.current->status.status = status;
+jobInfo = priv->job.current;
+if (status == QEMU_MONITOR_MIGRATION_STATUS_ERROR &&
+jobInfo->status.status == QEMU_MONITOR_MIGRATION_STATUS_CANCELLING) {
+VIR_DEBUG("State changed from \"cancelling\" to \"failed\"; setting "
+  "current state to \"cancelled\"");
+status = QEMU_MONITOR_MIGRATION_STATUS_CANCELLED;
+}
+
+jobInfo->status.status = status;
 virDomainObjSignal(vm);
 
  cleanup:
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 01/24] conf: Introduce per-domain condition variable

[Jiri Denemark]

Complex jobs, such as migration, need to monitor several events at once,
which is impossible when each of the event uses its own condition
variable. This patch adds a single condition variable to each domain
object. This variable can be used instead of the other event specific
conditions.

Signed-off-by: Jiri Denemark 
---

Notes:
ACKed in version 2

Version 3:
- rebased (context conflict in libvirt_private.syms)

Version 2:
- new patch which replaces thread queues and conditions (patch 1
  and 2 in version 1)

 src/conf/domain_conf.c   | 47 +++
 src/conf/domain_conf.h   |  6 ++
 src/libvirt_private.syms |  4 
 3 files changed, 57 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 36de844..433183f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2509,6 +2509,7 @@ static void virDomainObjDispose(void *obj)
 virDomainObjPtr dom = obj;
 
 VIR_DEBUG("obj=%p", dom);
+virCondDestroy(&dom->cond);
 virDomainDefFree(dom->def);
 virDomainDefFree(dom->newDef);
 
@@ -2529,6 +2530,12 @@ virDomainObjNew(virDomainXMLOptionPtr xmlopt)
 if (!(domain = virObjectLockableNew(virDomainObjClass)))
 return NULL;
 
+if (virCondInit(&domain->cond) < 0) {
+virReportSystemError(errno, "%s",
+ _("failed to initialize domain condition"));
+goto error;
+}
+
 if (xmlopt->privateData.alloc) {
 if (!(domain->privateData = (xmlopt->privateData.alloc)()))
 goto error;
@@ -2651,6 +2658,46 @@ virDomainObjEndAPI(virDomainObjPtr *vm)
 }
 
 
+void
+virDomainObjSignal(virDomainObjPtr vm)
+{
+virCondSignal(&vm->cond);
+}
+
+
+void
+virDomainObjBroadcast(virDomainObjPtr vm)
+{
+virCondBroadcast(&vm->cond);
+}
+
+
+int
+virDomainObjWait(virDomainObjPtr vm)
+{
+if (virCondWait(&vm->cond, &vm->parent.lock) < 0) {
+virReportSystemError(errno, "%s",
+ _("failed to wait for domain condition"));
+return -1;
+}
+return 0;
+}
+
+
+int
+virDomainObjWaitUntil(virDomainObjPtr vm,
+  unsigned long long whenms)
+{
+if (virCondWaitUntil(&vm->cond, &vm->parent.lock, whenms) < 0 &&
+errno != ETIMEDOUT) {
+virReportSystemError(errno, "%s",
+ _("failed to wait for domain condition"));
+return -1;
+}
+return 0;
+}
+
+
 /*
  *
  * If flags & VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE then
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ba17a8d..ac29ce5 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2318,6 +2318,7 @@ typedef struct _virDomainObj virDomainObj;
 typedef virDomainObj *virDomainObjPtr;
 struct _virDomainObj {
 virObjectLockable parent;
+virCond cond;
 
 pid_t pid;
 virDomainStateReason state;
@@ -2437,6 +2438,11 @@ void virDomainObjEndAPI(virDomainObjPtr *vm);
 bool virDomainObjTaint(virDomainObjPtr obj,
virDomainTaintFlags taint);
 
+void virDomainObjSignal(virDomainObjPtr vm);
+void virDomainObjBroadcast(virDomainObjPtr vm);
+int virDomainObjWait(virDomainObjPtr vm);
+int virDomainObjWaitUntil(virDomainObjPtr vm,
+  unsigned long long whenms);
 
 int virDomainDefCheckUnsupportedMemoryHotplug(virDomainDefPtr def);
 int virDomainDeviceDefCheckUnsupportedMemoryDevice(virDomainDeviceDefPtr dev);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 55a5e19..62a4b4c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -380,6 +380,7 @@ virDomainNetTypeToString;
 virDomainNostateReasonTypeFromString;
 virDomainNostateReasonTypeToString;
 virDomainObjAssignDef;
+virDomainObjBroadcast;
 virDomainObjCopyPersistentDef;
 virDomainObjEndAPI;
 virDomainObjFormat;
@@ -408,8 +409,11 @@ virDomainObjParseNode;
 virDomainObjSetDefTransient;
 virDomainObjSetMetadata;
 virDomainObjSetState;
+virDomainObjSignal;
 virDomainObjTaint;
 virDomainObjUpdateModificationImpact;
+virDomainObjWait;
+virDomainObjWaitUntil;
 virDomainOSTypeFromString;
 virDomainOSTypeToString;
 virDomainParseMemory;
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 21/24] qemu: Update migration state according to MIGRATION event

[Jiri Denemark]

We don't need to call query-migrate every 50ms when we get the current
migration state via MIGRATION event.

Signed-off-by: Jiri Denemark 
---

Notes:
ACKed in vesrion 2

Version 3:
- no change

Version 2:
- new patch

 src/qemu/qemu_migration.c | 14 --
 src/qemu/qemu_process.c   | 31 +++
 2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index d9f1a59..c3c2cac 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2511,7 +2511,11 @@ qemuMigrationCheckJobStatus(virQEMUDriverPtr driver,
 qemuDomainObjPrivatePtr priv = vm->privateData;
 qemuDomainJobInfoPtr jobInfo = priv->job.current;
 
-if (qemuMigrationUpdateJobStatus(driver, vm, asyncJob) < 0)
+bool events = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT);
+
+if (events)
+qemuMigrationUpdateJobType(jobInfo);
+else if (qemuMigrationUpdateJobStatus(driver, vm, asyncJob) < 0)
 return -1;
 
 switch (jobInfo->type) {
@@ -2530,9 +2534,15 @@ qemuMigrationCheckJobStatus(virQEMUDriverPtr driver,
qemuMigrationJobName(vm), _("canceled by client"));
 return -1;
 
+case VIR_DOMAIN_JOB_COMPLETED:
+/* Fetch statistics of a completed migration */
+if (events &&
+qemuMigrationUpdateJobStatus(driver, vm, asyncJob) < 0)
+return -1;
+break;
+
 case VIR_DOMAIN_JOB_BOUNDED:
 case VIR_DOMAIN_JOB_UNBOUNDED:
-case VIR_DOMAIN_JOB_COMPLETED:
 case VIR_DOMAIN_JOB_LAST:
 break;
 }
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ba84182..e703cbd 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1508,6 +1508,36 @@ qemuProcessHandleSpiceMigrated(qemuMonitorPtr mon 
ATTRIBUTE_UNUSED,
 }
 
 
+static int
+qemuProcessHandleMigrationStatus(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
+ virDomainObjPtr vm,
+ int status,
+ void *opaque ATTRIBUTE_UNUSED)
+{
+qemuDomainObjPrivatePtr priv;
+
+virObjectLock(vm);
+
+VIR_DEBUG("Migration of domain %p %s changed state to %s",
+  vm, vm->def->name,
+  qemuMonitorMigrationStatusTypeToString(status));
+
+priv = vm->privateData;
+if (priv->job.asyncJob != QEMU_ASYNC_JOB_MIGRATION_OUT &&
+priv->job.asyncJob != QEMU_ASYNC_JOB_MIGRATION_IN) {
+VIR_DEBUG("got MIGRATION event without a migration job");
+goto cleanup;
+}
+
+priv->job.current->status.status = status;
+virDomainObjSignal(vm);
+
+ cleanup:
+virObjectUnlock(vm);
+return 0;
+}
+
+
 static qemuMonitorCallbacks monitorCallbacks = {
 .eofNotify = qemuProcessHandleMonitorEOF,
 .errorNotify = qemuProcessHandleMonitorError,
@@ -1532,6 +1562,7 @@ static qemuMonitorCallbacks monitorCallbacks = {
 .domainNicRxFilterChanged = qemuProcessHandleNicRxFilterChanged,
 .domainSerialChange = qemuProcessHandleSerialChanged,
 .domainSpiceMigrated = qemuProcessHandleSpiceMigrated,
+.domainMigrationStatus = qemuProcessHandleMigrationStatus,
 };
 
 static int
-- 
2.4.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 22/24] qemu: Wait for migration events on domain condition

[Jiri Denemark]

Since we already support the MIGRATION event, we just need to make sure
the domain condition is signalled whenever a p2p connection drops or the
domain is paused due to IO error and we can avoid waking up every 50 ms
to check whether something happened.

Signed-off-by: Jiri Denemark 
---

Notes:
ACKed in version 2

Version 3:
- no change

Version 2:
- new patch

 src/qemu/qemu_domain.h|  3 +++
 src/qemu/qemu_migration.c | 45 +++--
 src/qemu/qemu_process.c   |  3 +++
 3 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 54e1e7b..86bd604 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -199,6 +199,9 @@ struct _qemuDomainObjPrivate {
 /* Bitmaps below hold data from the auto NUMA feature */
 virBitmapPtr autoNodeset;
 virBitmapPtr autoCpuset;
+
+bool signalIOError; /* true if the domain condition should be signalled on
+   I/O error */
 };
 
 # define QEMU_DOMAIN_DISK_PRIVATE(disk)\
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index c3c2cac..60c75f3 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2620,20 +2620,28 @@ qemuMigrationWaitForCompletion(virQEMUDriverPtr driver,
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
 qemuDomainJobInfoPtr jobInfo = priv->job.current;
+bool events = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT);
 int rv;
 
 jobInfo->type = VIR_DOMAIN_JOB_UNBOUNDED;
 while ((rv = qemuMigrationCompleted(driver, vm, asyncJob, dconn,
 abort_on_error, storage)) != 1) {
-/* Poll every 50ms for progress & to allow cancellation */
-struct timespec ts = { .tv_sec = 0, .tv_nsec = 50 * 1000 * 1000ull };
-
 if (rv < 0)
 return rv;
 
-virObjectUnlock(vm);
-nanosleep(&ts, NULL);
-virObjectLock(vm);
+if (events) {
+if (virDomainObjWait(vm) < 0) {
+jobInfo->type = VIR_DOMAIN_JOB_FAILED;
+return -2;
+}
+} else {
+/* Poll every 50ms for progress & to allow cancellation */
+struct timespec ts = { .tv_sec = 0, .tv_nsec = 50 * 1000 * 1000ull 
};
+
+virObjectUnlock(vm);
+nanosleep(&ts, NULL);
+virObjectLock(vm);
+}
 }
 
 qemuDomainJobInfoUpdateDowntime(jobInfo);
@@ -4050,6 +4058,7 @@ qemuMigrationRun(virQEMUDriverPtr driver,
 virErrorPtr orig_err = NULL;
 unsigned int cookieFlags = 0;
 bool abort_on_error = !!(flags & VIR_MIGRATE_ABORT_ON_ERROR);
+bool events = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT);
 int rc;
 
 VIR_DEBUG("driver=%p, vm=%p, cookiein=%s, cookieinlen=%d, "
@@ -4079,6 +4088,9 @@ qemuMigrationRun(virQEMUDriverPtr driver,
 return -1;
 }
 
+if (events)
+priv->signalIOError = abort_on_error;
+
 mig = qemuMigrationEatCookie(driver, vm, cookiein, cookieinlen,
  cookieFlags | QEMU_MIGRATION_COOKIE_GRAPHICS);
 if (!mig)
@@ -4284,6 +4296,9 @@ qemuMigrationRun(virQEMUDriverPtr driver,
 
 qemuMigrationCookieFree(mig);
 
+if (events)
+priv->signalIOError = false;
+
 if (orig_err) {
 virSetError(orig_err);
 virFreeError(orig_err);
@@ -4908,6 +4923,18 @@ doPeer2PeerMigrate3(virQEMUDriverPtr driver,
 }
 
 
+static void
+qemuMigrationConnectionClosed(virConnectPtr conn,
+  int reason,
+  void *opaque)
+{
+virDomainObjPtr vm = opaque;
+
+VIR_DEBUG("conn=%p, reason=%d, vm=%s", conn, reason, vm->def->name);
+virDomainObjSignal(vm);
+}
+
+
 static int virConnectCredType[] = {
 VIR_CRED_AUTHNAME,
 VIR_CRED_PASSPHRASE,
@@ -4981,6 +5008,11 @@ static int doPeer2PeerMigrate(virQEMUDriverPtr driver,
cfg->keepAliveCount) < 0)
 goto cleanup;
 
+if (virConnectRegisterCloseCallback(dconn, qemuMigrationConnectionClosed,
+vm, NULL) < 0) {
+goto cleanup;
+}
+
 qemuDomainObjEnterRemote(vm);
 p2p = VIR_DRV_SUPPORTS_FEATURE(dconn->driver, dconn,
VIR_DRV_FEATURE_MIGRATION_P2P);
@@ -5045,6 +5077,7 @@ static int doPeer2PeerMigrate(virQEMUDriverPtr driver,
  cleanup:
 orig_err = virSaveLastError();
 qemuDomainObjEnterRemote(vm);
+virConnectUnregisterCloseCallback(dconn, qemuMigrationConnectionClosed);
 virObjectUnref(dconn);
 qemuDomainObjExitRemote(vm);
 if (orig_err) {
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e703cbd..93c0844 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -952,6 +952,9 @@ qemuProcessHandleIOError(qemuMonitorPtr mon 
ATTRIBUTE_UNUSED,
 qemuDomainObjPrivatePt

[libvirt] [PATCH v3 19/24] qemu_monitor: Wire up MIGRATION event

[Jiri Denemark]

Thanks to Juan's work QEMU finally emits an event whenever migration
state changes.

Signed-off-by: Jiri Denemark 
---

Notes:
The MIGRATION event is not supported by QEMU yet, this patch is based
on the current patches available on qemu-devel mailing list. However,
there were no objections to the design of the event, which makes it
unlikely to change. Anyway this will have to wait until the patches
are applied to QEMU.

ACKed in version 2

Version 3:
- rebased (context conflict in qemu_capabilities.[ch])

Version 2:
- new patch

 src/qemu/qemu_capabilities.c |  3 +++
 src/qemu/qemu_capabilities.h |  1 +
 src/qemu/qemu_monitor.c  | 14 ++
 src/qemu/qemu_monitor.h  |  8 
 src/qemu/qemu_monitor_json.c | 23 +++
 5 files changed, 49 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index ca7a7c2..a80763a 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -285,6 +285,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "dea-key-wrap",
   "pci-serial",
   "aarch64-off",
+
+  "migration-event", /* 190 */
 );
 
 
@@ -1499,6 +1501,7 @@ struct virQEMUCapsStringFlags virQEMUCapsEvents[] = {
 { "BALLOON_CHANGE", QEMU_CAPS_BALLOON_EVENT },
 { "SPICE_MIGRATE_COMPLETED", QEMU_CAPS_SEAMLESS_MIGRATION },
 { "DEVICE_DELETED", QEMU_CAPS_DEVICE_DEL_EVENT },
+{ "MIGRATION", QEMU_CAPS_MIGRATION_EVENT },
 };
 
 struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index b5a7770..34cd078 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -229,6 +229,7 @@ typedef enum {
 QEMU_CAPS_DEA_KEY_WRAP   = 187, /* -machine dea_key_wrap */
 QEMU_CAPS_DEVICE_PCI_SERIAL  = 188, /* -device pci-serial */
 QEMU_CAPS_CPU_AARCH64_OFF= 189, /* -cpu ...,aarch64=off */
+QEMU_CAPS_MIGRATION_EVENT   = 190, /* MIGRATION event */
 
 QEMU_CAPS_LAST,   /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 94b0007..7f71a0e 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1491,6 +1491,20 @@ qemuMonitorEmitSpiceMigrated(qemuMonitorPtr mon)
 
 
 int
+qemuMonitorEmitMigrationStatus(qemuMonitorPtr mon,
+   int status)
+{
+int ret = -1;
+VIR_DEBUG("mon=%p, status=%s",
+  mon, NULLSTR(qemuMonitorMigrationStatusTypeToString(status)));
+
+QEMU_MONITOR_CALLBACK(mon, ret, domainMigrationStatus, mon->vm, status);
+
+return ret;
+}
+
+
+int
 qemuMonitorSetCapabilities(qemuMonitorPtr mon)
 {
 QEMU_CHECK_MONITOR(mon);
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 1afc344..7018045 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -186,6 +186,11 @@ typedef int 
(*qemuMonitorDomainSpiceMigratedCallback)(qemuMonitorPtr mon,
   virDomainObjPtr vm,
   void *opaque);
 
+typedef int (*qemuMonitorDomainMigrationStatusCallback)(qemuMonitorPtr mon,
+virDomainObjPtr vm,
+int status,
+void *opaque);
+
 typedef struct _qemuMonitorCallbacks qemuMonitorCallbacks;
 typedef qemuMonitorCallbacks *qemuMonitorCallbacksPtr;
 struct _qemuMonitorCallbacks {
@@ -214,6 +219,7 @@ struct _qemuMonitorCallbacks {
 qemuMonitorDomainNicRxFilterChangedCallback domainNicRxFilterChanged;
 qemuMonitorDomainSerialChangeCallback domainSerialChange;
 qemuMonitorDomainSpiceMigratedCallback domainSpiceMigrated;
+qemuMonitorDomainMigrationStatusCallback domainMigrationStatus;
 };
 
 char *qemuMonitorEscapeArg(const char *in);
@@ -313,6 +319,8 @@ int qemuMonitorEmitSerialChange(qemuMonitorPtr mon,
 const char *devAlias,
 bool connected);
 int qemuMonitorEmitSpiceMigrated(qemuMonitorPtr mon);
+int qemuMonitorEmitMigrationStatus(qemuMonitorPtr mon,
+   int status);
 
 int qemuMonitorStartCPUs(qemuMonitorPtr mon,
  virConnectPtr conn);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 0ba549e..1f070cf 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -84,6 +84,7 @@ static void qemuMonitorJSONHandleDeviceDeleted(qemuMonitorPtr 
mon, virJSONValueP
 static void qemuMonitorJSONHandleNicRxFilterChanged(qemuMonitorPtr mon, 
virJSONValuePtr data);
 static void qemuMonitorJSONHandleSerialChange(qemuMonitorPtr mon, 
virJSONValuePtr data);
 static void qemuMonitorJSONHandleSpiceMigrated(qemuMonit

[libvirt] [PATCH] qemu: fix cannot attach a virtio channel

[Luyao Huang]

https://bugzilla.redhat.com/show_bug.cgi?id=1230039

When attach a channel which target type is virtio, we will
get an error:

error: Failed to attach device from channel-file.xml
error: internal error: virtio serial device has invalid address type

This issue was introduced in commit 9807c4.
We didn't check the chr device type is serial then check
if the device target type is pci-serial, but not all the
Chr device is a serial type so we should check the device
type before check the target type to avoid assign wrong
address to other device type chr device.

Also most of chr device do not need {pci, virtio-serial} address
in qemu, we just get the address for the device which needed.

Signed-off-by: Luyao Huang 
---
 src/qemu/qemu_hotplug.c | 72 +++--
 1 file changed, 46 insertions(+), 26 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 3562de6..4d60513 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1531,6 +1531,48 @@ qemuDomainChrRemove(virDomainDefPtr vmdef,
 return ret;
 }
 
+static int
+qemuDomainAttachChrDeviceAssignAddr(qemuDomainObjPrivatePtr priv,
+virDomainChrDefPtr chr)
+{
+if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE &&
+chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO &&
+virDomainVirtioSerialAddrAutoAssign(NULL, priv->vioserialaddrs,
+&chr->info, true) < 0)
+return -1;
+
+if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
+chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI &&
+(chr->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
+ chr->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
+virDomainPCIAddressEnsureAddr(priv->pciaddrs, &chr->info) < 0)
+return -1;
+
+if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
+chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
+virDomainVirtioSerialAddrAutoAssign(NULL, priv->vioserialaddrs,
+&chr->info, false) < 0)
+return -1;
+
+return 0;
+}
+
+static void
+qemuDomainAttachChrDeviceReleaseAddr(qemuDomainObjPrivatePtr priv,
+ virDomainObjPtr vm,
+ virDomainChrDefPtr chr)
+{
+if ((chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE &&
+ chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) ||
+(chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
+ chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO))
+virDomainVirtioSerialAddrRelease(priv->vioserialaddrs, &chr->info);
+
+if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL &&
+chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI)
+qemuDomainReleaseDeviceAddress(vm, &chr->info, NULL);
+}
+
 int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
   virDomainObjPtr vm,
   virDomainChrDefPtr chr)
@@ -1541,7 +1583,6 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
 char *devstr = NULL;
 char *charAlias = NULL;
 bool need_release = false;
-bool allowZero = false;
 
 if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
 virReportError(VIR_ERR_OPERATION_INVALID, "%s",
@@ -1552,22 +1593,8 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
 if (qemuAssignDeviceChrAlias(vmdef, chr, -1) < 0)
 goto cleanup;
 
-if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE &&
-chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO)
-allowZero = true;
-
-if (chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI) {
-if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &chr->info) < 0)
-goto cleanup;
-} else if (chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB) {
-/* XXX */
-} else {
-if (virDomainVirtioSerialAddrAutoAssign(NULL,
-priv->vioserialaddrs,
-&chr->info,
-allowZero) < 0)
-goto cleanup;
-}
+if (qemuDomainAttachChrDeviceAssignAddr(priv, chr) < 0)
+goto cleanup;
 need_release = true;
 
 if (qemuBuildChrDeviceStr(&devstr, vm->def, chr, priv->qemuCaps) < 0)
@@ -1601,15 +1628,8 @@ int qemuDomainAttachChrDevice(virQEMUDriverPtr driver,
  cleanup:
 if (ret < 0 && virDomainObjIsActive(vm))
 qemuDomainChrInsertPreAllocCleanup(vm->def, chr);
-if (ret < 0 && need_release) {
-if (chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI) {
-qemuDomainReleaseDeviceAddress(vm, &chr->info, NULL);
-} else if (chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_USB) {
-  

Re: [libvirt] [PATCH v3 06/24] qemu: Abort migration early if disk mirror failed

[Peter Krempa]

On Wed, Jun 10, 2015 at 15:42:40 +0200, Jiri Denemark wrote:
> Abort migration as soon as we detect that some of the disk mirrors
> failed. There's no sense in trying to finish memory migration first.
> 
> Signed-off-by: Jiri Denemark 
> ---
> 
> Notes:
> Version 3:
> - new patch (separated from "qemu: Use domain condition for
>   synchronous block jobs")
> 
>  src/qemu/qemu_migration.c | 20 +---
>  1 file changed, 9 insertions(+), 11 deletions(-)
>

ACK,

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 v3 05/24] qemu: Cancel storage migration in parallel

[Peter Krempa]

On Wed, Jun 10, 2015 at 15:42:39 +0200, Jiri Denemark wrote:
> Instead of cancelling disk mirrors sequentially, let's just call
> block-job-cancel for all migrating disks and then wait until all
> disappear.
> 
> In case we cancel disk mirrors at the end of successful migration we
> also need to check all block jobs completed successfully. Otherwise we
> have to abort the migration.
> 
> Signed-off-by: Jiri Denemark 
> ---
> 

ACK,

Peter


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] virt-xml-validate: Allow input to be read from stdin

[Johannes Holmberg]

---

Hello,

I often find myself wanting to validate a domain xml without having
to save it to a file, so I've updated virt-xml-validate to support
the standard practice of reading from stdin when "-" is given as the
input file.
Of course, the validation is multipass so there still needs to be a
temporary file but I think it makes sense to hide this complexity in
virt-xml-validate and not force it onto the user.

 /Johannes
 

 tools/virt-xml-validate.in | 53 +-
 1 file changed, 38 insertions(+), 15 deletions(-)

diff --git a/tools/virt-xml-validate.in b/tools/virt-xml-validate.in
index a04fa06..8807f8d 100644
--- a/tools/virt-xml-validate.in
+++ b/tools/virt-xml-validate.in
@@ -17,6 +17,16 @@
 
 set -e
 
+TMPFILE=
+
+cleanup() {
+  if [ $TMPFILE ]; then
+rm -f $TMPFILE
+  fi
+}
+
+trap cleanup EXIT
+
 case $1 in
   -h | --h | --he | --hel | --help)
 cat <&2
 exit 1 ;;
 esac
@@ -43,18 +53,27 @@ esac
 XMLFILE="$1"
 TYPE="$2"
 
-if [ -z "$XMLFILE" ]; then
-  echo "syntax: $0 XMLFILE [TYPE]" >&2
-  exit 1
-fi
-
-if [ ! -f "$XMLFILE" ]; then
-  echo "$0: document $XMLFILE does not exist" >&2
-  exit 2
+if [ "$XMLFILE" = "-" ]; then
+TMPFILE=`mktemp --tmpdir virt-xml.`
+cat > $TMPFILE
+else
+  if [ -z "$XMLFILE" ]; then
+echo "syntax: $0 XMLFILE [TYPE]" >&2
+exit 1
+  fi
+
+  if [ ! -f "$XMLFILE" ]; then
+echo "$0: document $XMLFILE does not exist" >&2
+exit 2
+  fi
 fi
 
 if [ -z "$TYPE" ]; then
-  ROOT=`xmllint --stream --debug "$XMLFILE" 2>/dev/null | grep "^0 1 " | awk 
'{ print $3 }'`
+  if [ $TMPFILE ]; then
+ROOT=`cat "$TMPFILE" | xmllint --stream --debug - 2>/dev/null | grep "^0 1 
" | awk '{ print $3 }'`
+  else
+ROOT=`xmllint --stream --debug "$XMLFILE" 2>/dev/null | grep "^0 1 " | awk 
'{ print $3 }'`
+  fi
   case "$ROOT" in
  *domainsnapshot*) # Must come first, since *domain* is a substring
 TYPE="domainsnapshot"
@@ -99,8 +118,11 @@ if [ ! -f "$SCHEMA" ]; then
   exit 4
 fi
 
-xmllint --noout --relaxng "$SCHEMA" "$XMLFILE"
-
+if [ $TMPFILE ]; then
+  cat "$TMPFILE" | xmllint --noout --relaxng "$SCHEMA" -
+else
+  xmllint --noout --relaxng "$SCHEMA" "$XMLFILE"
+fi
 exit
 
 : <<=cut
@@ -119,9 +141,10 @@ exit
 
 Validates a libvirt XML for compliance with the published schema.
 The first compulsory argument is the path to the XML file to be
-validated. The optional second argument is the name of the schema
-to validate against. If omitted, the schema name will be inferred
-from the name of the root element in the XML document.
+validated (or - to read the XML from standard input). The optional
+second argument is the name of the schema to validate against. If
+omitted, the schema name will be inferred from the name of the root
+element in the XML document.
 
 Valid schema names currently include
 
-- 
1.9.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 00/24] Add support for migration events

[Jiri Denemark]

On Wed, Jun 10, 2015 at 10:27:11 -0400, John Ferlan wrote:
> 
> 
> On 06/10/2015 09:42 AM, Jiri Denemark wrote:
> > QEMU will soon (patches are available on qemu-devel) get support for
> > migration events which will finally allow us to get rid of polling
> > query-migrate every 50ms. However, we first need to be able to wait for
> > all events related to migration (migration status changes, block job
> > events, async abort requests) at once. This series prepares the
> > infrastructure and uses it to switch all polling loops in migration code
> > to pthread_cond_wait.
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1212077
> > 
> > Version 3 (see individual patches for details):
> > - most of the series has been ACKed in v2
> > - "qemu: Use domain condition for synchronous block jobs" was split in 3
> >   patches for easier review
> > - minor changes requested in v2 review
> > 
> > Version 2 (see individual patches for details):
> > - rewritten using per-domain condition variable
> > - enahnced to fully support the migration events
> 
> Just ran this through my Coverity checker - only one issue
> 
> RESOURCE_LEAK in qemuMigrationRun:
> 
> 4235  if (qemuMigrationCheckJobStatus(driver, vm,
> 4236  QEMU_ASYNC_JOB_MIGRATION_OUT) < 
> 0)
> 
> (4) Event if_end: End of if statement
> 
> 4237  goto cancel;
> 4238  
> 
> (5) Event open_fn:Returning handle opened by "accept".
> (6) Event var_assign: Assigning: "fd" = handle returned from 
> "accept(spec->dest.unix_socket.sock, __SOCKADDR_ARG({ .__sockaddr__ = NULL}), 
> NULL)".
> (7) Event cond_false: Condition "(fd = 
> accept(spec->dest.unix_socket.sock, __SOCKADDR_ARG({ .__sockaddr__ = NULL}), 
> NULL)) < 0", taking false branch
> Also see events:  [leaked_handle]
> 
> 4239  while ((fd = accept(spec->dest.unix_socket.sock, NULL, NULL)) < 
> 0) {
> 4240  if (errno == EAGAIN || errno == EINTR)
> 4241  continue;

Hmm, what an old and unused (except for some ancient QEMU versions) code
path :-) However, this code is only executed if spec->destType ==
MIGRATION_DEST_UNIX, which only happens in tunnelled migration path,
which also sets spec.fwdType = MIGRATION_FWD_STREAM.

...
> (28) Event cond_false:Condition "spec->fwdType != 
> MIGRATION_FWD_DIRECT", taking false branch
> 
> 4289  if (spec->fwdType != MIGRATION_FWD_DIRECT) {
> 4290  if (iothread && qemuMigrationStopTunnel(iothread, ret < 0) < 0)
> 4291  ret = -1;
> 4292  VIR_FORCE_CLOSE(fd);

Which means "spec->fwdType != MIGRATION_FWD_DIRECT" will be true and fd
will be correctly closed.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 04/24] qemu: Use domain condition for synchronous block jobs

[Peter Krempa]

On Wed, Jun 10, 2015 at 15:42:38 +0200, Jiri Denemark wrote:
> By switching block jobs to use domain conditions, we can drop some
> pretty complicated code in NBD storage migration.
> 
> Signed-off-by: Jiri Denemark 
> ---
> 
> Notes:
> Version 3:
> - split into 3 patches
> 
> Version 2:
> - slightly modified to use domain conditions
> 
>  po/POTFILES.in|   1 -
>  src/qemu/qemu_blockjob.c  | 137 
> +++---
>  src/qemu/qemu_blockjob.h  |  12 +---
>  src/qemu/qemu_domain.c|  17 +-
>  src/qemu/qemu_domain.h|   1 -
>  src/qemu/qemu_driver.c|  24 
>  src/qemu/qemu_migration.c | 112 +
>  src/qemu/qemu_process.c   |  13 ++---
>  8 files changed, 76 insertions(+), 241 deletions(-)
> 

ACK,

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 v3 00/24] Add support for migration events

[Peter Krempa]

On Wed, Jun 10, 2015 at 15:42:34 +0200, Jiri Denemark wrote:
> QEMU will soon (patches are available on qemu-devel) get support for
> migration events which will finally allow us to get rid of polling
> query-migrate every 50ms. However, we first need to be able to wait for
> all events related to migration (migration status changes, block job
> events, async abort requests) at once. This series prepares the
> infrastructure and uses it to switch all polling loops in migration code
> to pthread_cond_wait.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1212077
> 
> Version 3 (see individual patches for details):
> - most of the series has been ACKed in v2
> - "qemu: Use domain condition for synchronous block jobs" was split in 3
>   patches for easier review
> - minor changes requested in v2 review
> 
> Version 2 (see individual patches for details):
> - rewritten using per-domain condition variable
> - enahnced to fully support the migration events
> 
> 
> Jiri Denemark (24):
>   conf: Introduce per-domain condition variable
>   qemu: Introduce qemuBlockJobUpdate
>   qemu: Properly report failed migration
>   qemu: Use domain condition for synchronous block jobs
>   qemu: Cancel storage migration in parallel
>   qemu: Abort migration early if disk mirror failed
>   qemu: Don't mess with disk->mirrorState
>   Pass domain object to private data formatter/parser
>   qemu: Make qemuMigrationCancelDriveMirror usable without async job
>   qemu: Refactor qemuMonitorBlockJobInfo
>   qemu: Cancel disk mirrors after libvirtd restart
>   qemu: Use domain condition for asyncAbort
>   qemu_monitor: Wire up SPICE_MIGRATE_COMPLETED event
>   qemu: Do not poll for spice migration status
>   qemu: Refactor qemuDomainGetJob{Info,Stats}
>   qemu: Refactor qemuMigrationUpdateJobStatus
>   qemu: Don't pass redundant job name around
>   qemu: Refactor qemuMigrationWaitForCompletion
>   qemu_monitor: Wire up MIGRATION event
>   qemuDomainGetJobStatsInternal: Support migration events
>   qemu: Update migration state according to MIGRATION event
>   qemu: Wait for migration events on domain condition
>   qemu: cancel drive mirrors when p2p connection breaks

ACK to the above ones once qemu accepts the event stuff.

Peter

>   DO NOT APPLY: qemu: Work around weird migration status changes


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v3 00/24] Add support for migration events

[John Ferlan]

On 06/10/2015 11:06 AM, Jiri Denemark wrote:
> On Wed, Jun 10, 2015 at 10:27:11 -0400, John Ferlan wrote:
>>
>>
>> On 06/10/2015 09:42 AM, Jiri Denemark wrote:
>>> QEMU will soon (patches are available on qemu-devel) get support for
>>> migration events which will finally allow us to get rid of polling
>>> query-migrate every 50ms. However, we first need to be able to wait for
>>> all events related to migration (migration status changes, block job
>>> events, async abort requests) at once. This series prepares the
>>> infrastructure and uses it to switch all polling loops in migration code
>>> to pthread_cond_wait.
>>>
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1212077
>>>
>>> Version 3 (see individual patches for details):
>>> - most of the series has been ACKed in v2
>>> - "qemu: Use domain condition for synchronous block jobs" was split in 3
>>>   patches for easier review
>>> - minor changes requested in v2 review
>>>
>>> Version 2 (see individual patches for details):
>>> - rewritten using per-domain condition variable
>>> - enahnced to fully support the migration events
>>
>> Just ran this through my Coverity checker - only one issue
>>
>> RESOURCE_LEAK in qemuMigrationRun:
>>
>> 4235 if (qemuMigrationCheckJobStatus(driver, vm,
>> 4236 
>> QEMU_ASYNC_JOB_MIGRATION_OUT) < 0)
>>
>> (4) Event if_end:End of if statement
>>
>> 4237 goto cancel;
>> 4238 
>>
>> (5) Event open_fn:   Returning handle opened by "accept".
>> (6) Event var_assign:Assigning: "fd" = handle returned from 
>> "accept(spec->dest.unix_socket.sock, __SOCKADDR_ARG({ .__sockaddr__ = 
>> NULL}), NULL)".
>> (7) Event cond_false:Condition "(fd = 
>> accept(spec->dest.unix_socket.sock, __SOCKADDR_ARG({ .__sockaddr__ = NULL}), 
>> NULL)) < 0", taking false branch
>> Also see events: [leaked_handle]
>>
>> 4239 while ((fd = accept(spec->dest.unix_socket.sock, NULL, 
>> NULL)) < 0) {
>> 4240 if (errno == EAGAIN || errno == EINTR)
>> 4241 continue;
> 
> Hmm, what an old and unused (except for some ancient QEMU versions) code
> path :-) However, this code is only executed if spec->destType ==
> MIGRATION_DEST_UNIX, which only happens in tunnelled migration path,
> which also sets spec.fwdType = MIGRATION_FWD_STREAM.
> 

Placing "sa_assert(spec->fwdType == MIGRATION_FWD_STREAM);" above the
while loop makes Coverity happy.

John
> ...
>> (28) Event cond_false:   Condition "spec->fwdType != 
>> MIGRATION_FWD_DIRECT", taking false branch
>>
>> 4289 if (spec->fwdType != MIGRATION_FWD_DIRECT) {
>> 4290 if (iothread && qemuMigrationStopTunnel(iothread, ret < 
>> 0) < 0)
>> 4291 ret = -1;
>> 4292 VIR_FORCE_CLOSE(fd);
> 
> Which means "spec->fwdType != MIGRATION_FWD_DIRECT" will be true and fd
> will be correctly closed.
> 
> Jirka
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Fix virsh edit and virt-xml-validate with SCSI LUN

[Eric Farman]

Defining a domain with a SCSI disk attached via a hostdev
tag and a source address unit value longer than two digits
causes an error when editing the domain with virsh edit,
even if no changes are made to the domain definition.
The error suggests invalid XML, somewhere:

  # virsh edit lmb_guest
  error: XML document failed to validate against schema:
  Unable to validate doc against /usr/local/share/libvirt/schemas/domain.rng
  Extra element devices in interleave
  Element domain failed to validate content

The virt-xml-validate tool fails with a similar error:

  # virt-xml-validate lmb_guest.xml
  Relax-NG validity error : Extra element devices in interleave
  lmb_guest.xml:17: element devices: Relax-NG validity error :
  Element domain failed to validate content
  lmb_guest.xml fails to validate

The hostdev tag requires a source address to be specified,
which includes bus, target, and unit address attributes.
According to the SCSI Architecture Model spec (section
4.9 of SAM-2), a LUN address is 64 bits and thus could be
up to 20 decimal digits long.  Unfortunately, the XML
schema limits this string to just two digits.  Similarly,
the target field can be up to 32 bits in length, which
would be 10 decimal digits.

  # lsscsi -xx
  [0:0:19:0x40224011]  diskIBM  2107900  3.44 /dev/sda
  # lsscsi
  [0:0:19:1074872354]diskIBM  2107900  3.44  /dev/sda
  # cat lmb_guest.xml
  
lmb_guest
1024
  ...trimmed...

  
  

  
  

  
  ...trimmed...

Since the reference unit and target fields are used in
several places in the XML schema, create a separate one
specific for SCSI Logical Units that will permit the
greater length.  Also, expand the definition of the SCSI
unit field from an int to a long long, to cover the
possible size. This permits both the validation utility
and the virsh edit command to succeed when a hostdev
tag is included.

Signed-off-by: Eric Farman 
Reviewed-by: Matthew Rosato 
Reviewed-by: Stefan Zimmermann 
Reviewed-by: Boris Fiuczynski 
---
 docs/formatdomain.html.in | 10 +++---
 docs/schemas/domaincommon.rng | 14 --
 src/conf/domain_audit.c   |  2 +-
 src/conf/domain_conf.c|  4 ++--
 src/conf/domain_conf.h|  2 +-
 src/qemu/qemu_command.h   |  2 +-
 src/qemu/qemu_hotplug.c   |  4 ++--
 src/util/virhostdev.c |  6 +++---
 src/util/virscsi.c| 16 
 src/util/virscsi.h|  8 
 tests/testutilsqemu.c |  2 +-
 tools/virsh-domain.c  |  6 +++---
 12 files changed, 45 insertions(+), 31 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 1781996..e7a8e1a 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2827,7 +2827,7 @@
   Drive addresses have the following additional
 attributes: controller (a 2-digit controller
 number), bus (a 2-digit bus number),
-target (a 2-digit bus number),
+target (a 2-digit target number),
 and unit (a 2-digit unit number on the bus).
   
   type='virtio-serial'
@@ -3136,7 +3136,7 @@
 
   
 
-

+

   
   
   

@@ -3244,7 +3244,11 @@
   
   scsi
   SCSI devices are described by both the adapter
-and address elements.
+and address elements. The address
+element includes a bus attribute (a 2-digit bus
+number), a target attribute (a 10-digit target
+number), and a unit attribute (a 20-digit unit
+number on the bus).
 
 Since 1.2.8, the source
 element of a SCSI device may contain the protocol
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 5dc48f7..dccd3fd 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3846,10 +3846,10 @@
   
 
 
-  
+  
 
 
-  
+  
 
   
   
@@ -5142,11 +5142,21 @@
   [0-9]{1,2}
 
   
+  
+
+  [0-9]{1,10}
+
+  
   
 
   [0-9]{1,2}
 
   
+  
+
+  [0-9]{1,20}
+
+  
   
 
   [a-zA-Z0-9\-_\.]+
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index 1900039..844297c 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -427,7 +427,7 @@ virDomainAuditHostdev(virDomainObjPtr vm, 
virDomainHostdevDefPtr hostdev,
 } else {
 virDomainHostdevSubsysSCSIHostPtr scsihostsrc =
 &scsisrc->u.host;
-if (virAsprintfQuiet(&address, "%s:%d:%d:%d",
+if (virAspri

Re: [libvirt] [PATCH v3 19/24] qemu_monitor: Wire up MIGRATION event

[Peter Krempa]

On Wed, Jun 10, 2015 at 15:42:53 +0200, Jiri Denemark wrote:
> Thanks to Juan's work QEMU finally emits an event whenever migration
> state changes.
> 
> Signed-off-by: Jiri Denemark 
> ---
> 
> Notes:
> The MIGRATION event is not supported by QEMU yet, this patch is based
> on the current patches available on qemu-devel mailing list. However,
> there were no objections to the design of the event, which makes it
> unlikely to change. Anyway this will have to wait until the patches
> are applied to QEMU.
> 
> ACKed in version 2
> 
> Version 3:
> - rebased (context conflict in qemu_capabilities.[ch])
> 
> Version 2:
> - new patch
> 
>  src/qemu/qemu_capabilities.c |  3 +++
>  src/qemu/qemu_capabilities.h |  1 +
>  src/qemu/qemu_monitor.c  | 14 ++
>  src/qemu/qemu_monitor.h  |  8 
>  src/qemu/qemu_monitor_json.c | 23 +++
>  5 files changed, 49 insertions(+)

...

> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index b5a7770..34cd078 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -229,6 +229,7 @@ typedef enum {
>  QEMU_CAPS_DEA_KEY_WRAP   = 187, /* -machine dea_key_wrap */
>  QEMU_CAPS_DEVICE_PCI_SERIAL  = 188, /* -device pci-serial */
>  QEMU_CAPS_CPU_AARCH64_OFF= 189, /* -cpu ...,aarch64=off */
> +QEMU_CAPS_MIGRATION_EVENT = 190, /* MIGRATION event */

The alignment of the equals sign is off here.

>  
>  QEMU_CAPS_LAST,   /* this must always be the last item */
>  } virQEMUCapsFlags;

ACK with that fixed.

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] util: Fix coverity warings in virProcessGetAffinity

[John Ferlan]

On 06/10/2015 09:41 AM, Peter Krempa wrote:
> On Thu, Jun 04, 2015 at 08:34:00 -0400, John Ferlan wrote:
>> On 06/04/2015 08:15 AM, Peter Krempa wrote:
>>> Refactor the code flow a bit more to clear coverity errors.
>>>
>>> Store the cpu count in an intermediate variable and reuse it rather than
>>> caluclating the index.
>>> ---
>>>  src/util/virprocess.c | 11 +++
>>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>>
> 
> ...
> 
>>
>> ^^^ Coverity still complains here
>>
>> No real change since previous...
>>

nice detective work...

> 
> So I went in and traced it a bit. cpu_set_t is basically the following
> structure: (assuming sizeof(unsigned long int) == 8))
> 
> typedef struct
> {
>   unsigned long int __bits[1024 / 8 * 8];
> } cpu_set_t;
> 
> 
> The CPU_ALLOC() macro allocates an array of such structures. Since the
> size of the __bits array is rounded nicely (128 bytes) the structure
> itself does not add any padding and it's size is 128 bytes too the
> structures are packed and thus the __bits fields of adjecent structures
> basically form a longer array of unsigned long ints.
> 
> The CPU_ISSET_S macro then translates basically to this equivalent
> function:
> 
> unsigned long int
> CPU_ISSET_S(size_t __cpu,
> size_t setsize,
> cpu_set_t *cpusetp)
> {
> if (__cpu / 8 < setsize) {
> unsigned long int subcpu =  ((const unsigned long int 
> *)(mask->__bits))[__cpu / 64];
> unsigned long int mask = (unsigned long int) 1 << (__cpu % 64);
> 
> return subcpu & mask;
> } else {
> return 0;
> }
> }
> 
> The macro expects that the array is packed nicely and treats it
> basically as a large array of unsigned ints. The macro then selects one
> of the members and masks out the required cpu bit.
> 
> So then compiling the following proof of concept:
> 
> #define _GNU_SOURCE
> #include 
> 
> int main(void)
> {
> size_t ncpus = 1024 << 8;
> size_t masklen = CPU_ALLOC_SIZE(ncpus);
> cpu_set_t *mask = CPU_ALLOC(ncpus);
> 
> CPU_ZERO_S(masklen, mask);
> 
> for (size_t i = 0; i < ncpus; i++) {
> if (CPU_ISSET_S(i, masklen, mask)) {
> i = i;
> }
> }
> CPU_FREE(mask);
> 
> return 0;
> }
> 
> And running it in valgrind results in no error as expected:
> 
> $ valgrind ./a.out 
> ==95609== Memcheck, a memory error detector
> ==95609== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
> ==95609== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
> ==95609== Command: ./a.out
> ==95609== 
> ==95609== 
> ==95609== HEAP SUMMARY:
> ==95609== in use at exit: 0 bytes in 0 blocks
> ==95609==   total heap usage: 1 allocs, 1 frees, 32,768 bytes allocated
> ==95609== 
> ==95609== All heap blocks were freed -- no leaks are possible
> ==95609== 
> ==95609== For counts of detected and suppressed errors, rerun with: -v
> ==95609== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
> 
> The problem is that coverity thinks that the code overruns the __bits
> array now that the code is simple enough to introspect which is
> technically true. Previously the same situation would happen but the
> loop that would resize the array incrementally probably was not
> introspected properly and thus did not produce a warning.
> 
> So there are basically three options:
> 1) Silence the coverity warning

So on line just prior to CPU_ISSET_S either:

sa_assert(sizeof(unsigned long int) == sizeof(cpu_set_t));

or

/* coverity[overrun-local] */

Silences the warning

ACK for the current adjustment - your call if you want the sa_assert or
the coverity noise.

John

> 2) File a bug against libc or something to fix the macro
> 3) Reimplement CPU_ISSET_S internally. (Which I don't think will be
> possible since cpu_set_t does not look public)
> 
> At any rate, there is no problem in libvirt's usage as it conforms to
> the example in the man page.
> 
> As of this particular patch. It does not fix the coverity warning, but I
> think the intention and code flow is more apparent once it's applied.
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCHv2] qemu: add a check for slot and base when build dimm address

[John Ferlan]

On 05/27/2015 05:50 AM, Luyao Huang wrote:
> When hot-plug a memory device, we don't check if there
> is a memory device have the same address with the memory device
> we want hot-pluged. Qemu forbid use/hot-plug 2 memory device
> with same slot or the same base(qemu side this elemnt named addr).
> 
> Introduce a address check when build memory device qemu command line.
> 
> Signed-off-by: Luyao Huang 
> ---
> v2:
>  move the check to qemuBuildMemoryDeviceStr() to check the dimm address
>  when start/hot-plug a memory device.
> 
>  src/qemu/qemu_command.c | 77 
> -
>  1 file changed, 57 insertions(+), 20 deletions(-)
> 

Perhaps a bit easier to review if this was split into two patches the
first being purely code motion and the second being fixing the
problem...  That said, I don't think the refactor is necessary. I've
attached an alternative and some notes inline below...

John

> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index d8ce511..0380a3b 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4955,6 +4955,61 @@ qemuBuildMemoryDimmBackendStr(virDomainMemoryDefPtr 
> mem,
>  }
>  
>  
> +static int
> +qemuBuildMemoryDeviceAddr(virBuffer *buf,
> +  virDomainDefPtr def,
> +  virDomainMemoryDefPtr mem)

static bool
qemuCheckMemoryDimmConflict(virDomainDefPtr def,
virDomainMemoryDefPtr mem)

> +{
> +ssize_t i;

size_t usually, then keep the following checks in the caller

> +
> +if (mem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
> +return 0;
> +
> +if (mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   _("only 'dimm' addresses are supported for the "
> + "pc-dimm device"));
> +return -1;
> +}
> +
> +if (mem->info.addr.dimm.slot >= def->mem.memory_slots) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +   _("memory device slot '%u' exceeds slots count '%u'"),
> +   mem->info.addr.dimm.slot, def->mem.memory_slots);
> +return -1;
> +}
> +

Thru here... Since it seems the following is your bugfix correct?

> +for (i = 0; i < def->nmems; i++) {
> + virDomainMemoryDefPtr tmp = def->mems[i];
> +
> + if (tmp == mem ||
> + tmp->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM)
> + continue;
> +
> + if (mem->info.addr.dimm.slot == tmp->info.addr.dimm.slot) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +_("memory device slot '%u' already been"
> +  " used by other memory device"),

...is already being used by another

> +mem->info.addr.dimm.slot);
> + return -1;

return true;

> + }
> +
> + if (mem->info.addr.dimm.base != 0 && tmp->info.addr.dimm.base != 0 
> &&
> + mem->info.addr.dimm.base == tmp->info.addr.dimm.base) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +_("memory device base '0x%llx' already been"

...is already being used by another...

> +  " used by other memory device"),
> +mem->info.addr.dimm.base);
> + return -1;

return true

> + }
> +}

return false;

Keep remainder in caller

> +
> +virBufferAsprintf(buf, ",slot=%d", mem->info.addr.dimm.slot);
> +virBufferAsprintf(buf, ",addr=%llu", mem->info.addr.dimm.base);
> +
> +return 0;
> +}
> +
>  char *
>  qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem,
>   virDomainDefPtr def,
> @@ -4976,29 +5031,11 @@ qemuBuildMemoryDeviceStr(virDomainMemoryDefPtr mem,
>  return NULL;
>  }
>  
> -if (mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM &&
> -mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -   _("only 'dimm' addresses are supported for the "
> - "pc-dimm device"));

Your refactor adjusts this test, so if the type was something else then
the virBufferAsprintf happens before the error... it's a nit, but future
adjustments could do something unexpected...

> -return NULL;
> -}
> -
> -if (mem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DIMM &&
> -mem->info.addr.dimm.slot >= def->mem.memory_slots) {
> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -   _("memory device slot '%u' exceeds slots count 
> '%u'"),
> -   mem->info.addr.dimm.slot, def->mem.memory_slots);
> -return NULL;
> -}
> -

Rather than refactoring - why not change the purpos

Re: [libvirt] [PATCH] network: add an option to make dns public

[John Ferlan]

On 06/01/2015 07:54 AM, Cédric Bosdonnat wrote:
> In some use cases we don't want the virtual network's DNS to only
> listen to the vnet interface. Adding a publiclyAccessible attribute
> to the dns element in the configuration allows the DNS to listen to
> all interfaces.
> 
> It simply disables the bind-dynamic option of dnsmasq for the network.
> ---
>  docs/formatnetwork.html.in   | 11 +++
>  docs/schemas/network.rng | 15 ++-
>  src/conf/network_conf.c  |  6 ++
>  src/conf/network_conf.h  |  1 +
>  src/network/bridge_driver.c  |  4 +++-
>  tests/networkxml2confdata/nat-network-dns-hosts.conf |  1 -
>  tests/networkxml2confdata/nat-network-dns-hosts.xml  |  2 +-
>  7 files changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
> index 6abed8f..8e43658 100644
> --- a/docs/formatnetwork.html.in
> +++ b/docs/formatnetwork.html.in
> @@ -851,6 +851,17 @@
>DNS server.
>  
>  
> +
> +  The dns element
> +  can have an optional publiclyAccessible
> +  attribute Since 1.2.17.
> +  If publiclyAccessible is "yes", then the DNS server
> +  will handle requests for all interfaces.
> +  If publiclyAccessible is not set or "no", the DNS
> +  server will only handle requests for the interface of the virtual
> +  network.
> +
> +
>  Currently supported sub-elements of  are:
>  
>forwarder
> diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
> index 4edb6eb..f989625 100644
> --- a/docs/schemas/network.rng
> +++ b/docs/schemas/network.rng
> @@ -244,12 +244,17 @@
>   and other features in the  element -->
>  
>
> -
> -  
> -
> -  
> -
>  
> +  
> +
> +  
> +
> +  
> +  
> +
> +  
> +
> +  

Moving the attributes inside the  had me looking through
other .rng's... I'm no expert, but had thought they really only mattered
for 's

>
>  
>
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index f4a9df0..99bac6d 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -1309,9 +1309,14 @@ virNetworkDNSDefParseXML(const char *networkName,
>  size_t i;
>  int ret = -1;
>  xmlNodePtr save = ctxt->node;
> +char *publiclyAccessible = NULL;
>  
>  ctxt->node = node;
>  
> +publiclyAccessible = virXPathString("string(./@publiclyAccessible)", 
> ctxt);
> +if (publiclyAccessible)
> +def->publiclyAccessible = 
> virTristateBoolTypeFromString(publiclyAccessible);
> +
>  forwardPlainNames = virXPathString("string(./@forwardPlainNames)", ctxt);
>  if (forwardPlainNames) {
>  def->forwardPlainNames = 
> virTristateBoolTypeFromString(forwardPlainNames);
> @@ -1410,6 +1415,7 @@ virNetworkDNSDefParseXML(const char *networkName,
>  
>  ret = 0;
>   cleanup:
> +VIR_FREE(publiclyAccessible);
>  VIR_FREE(forwardPlainNames);
>  VIR_FREE(fwdNodes);
>  VIR_FREE(hostNodes);
> diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
> index f69d999..f555b6b 100644
> --- a/src/conf/network_conf.h
> +++ b/src/conf/network_conf.h
> @@ -136,6 +136,7 @@ struct _virNetworkDNSDef {
>  virNetworkDNSSrvDefPtr srvs;
>  size_t nfwds;
>  char **forwarders;
> +int publiclyAccessible; /* enum virTristateBool */
>  };
>  
>  typedef struct _virNetworkIpDef virNetworkIpDef;
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index d195085..c39b1a5 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -996,8 +996,10 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
>   * other than one of the virtual guests connected directly to
>   * this network). This was added in response to CVE 2012-3411.
>   */
> +if (network->def->dns.publiclyAccessible != VIR_TRISTATE_BOOL_YES)
> +virBufferAddLit(&configbuf,
> +  "bind-dynamic\n");
>  virBufferAsprintf(&configbuf,
> -  "bind-dynamic\n"
>"interface=%s\n",
>network->def->bridge);
>  } else {
> diff --git a/tests/networkxml2confdata/nat-network-dns-hosts.conf 
> b/tests/networkxml2confdata/nat-network-dns-hosts.conf
> index 021316f..759a9e9 100644
> --- a/tests/networkxml2confdata/nat-network-dns-hosts.conf
> +++ b/tests/networkxml2confdata/nat-network-dns-hosts.conf
> @@ -10,6 +10,5 @@ expand-hosts
>  domain-needed
>  local=//
>  

Re: [libvirt] [PATCH v2 0/2] lxc: properly clean up qemu-nbd

[John Ferlan]

On 06/01/2015 09:01 AM, Cédric Bosdonnat wrote:
> Hi all,
> 
> Here is the very same patch, but split in two patches. Well, I also moved
> two comments around between v1 and v2.
> 
> Cédric Bosdonnat (2):
>   Add virProcessGetPids to get all tasks of a process
>   lxc: properly clean up qemu-nbd
> 
>  src/libvirt_private.syms |  1 +
>  src/lxc/lxc_controller.c | 56 
> 
>  src/util/virprocess.c| 47 
>  src/util/virprocess.h|  2 ++
>  4 files changed, 106 insertions(+)
> 

Never saw the 1/2 and 2/2 show up in my inbox and I don't see them in
the archive - just your 0/2.

Could you send again?

Tks -

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Canging bridge names on live migration

[Philipp Hahn]

Hello,

On 03.06.2015 13:18, seitan wrote:
> i wonder, if there's a possibility to change a name of a shared interface in
> virtual machine config, while doing migration.
> The problem is:
> hypervisor1 (source) uses shared interface name  "br0".
> hypervisor2 (target) uses shared interface name "br500".
> Live migration fails, because target hypervisor does not have "br0" interface.

libvirt supports some hook scripts, which can be used to re-write the
XML file during live migration. Have a look at
, which
should work similarly for your scenario.

Philipp

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] maint: document use of zanata for translations

[John Ferlan]

On 05/27/2015 10:43 AM, Eric Blake wrote:
> Based on recent list questions on how to contribute a translation fix.
> 
> Signed-off-by: Eric Blake 
> ---
> 
> Should be safe for freeze, but as I have never contributed a
> translation fix, I'll wait for review.
> 
>  HACKING  | 19 ---
>  docs/hacking.html.in |  7 +++
>  2 files changed, 19 insertions(+), 7 deletions(-)
> 

Rather than see it languish any longer... I think the process you've
described here fits stuff I've read recently regarding this process, so

ACK

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Canging bridge names on live migration

[Laine Stump]

On 06/10/2015 02:06 PM, Philipp Hahn wrote:
> Hello,
>
> On 03.06.2015 13:18, seitan wrote:
>> i wonder, if there's a possibility to change a name of a shared interface in
>> virtual machine config, while doing migration.
>> The problem is:
>> hypervisor1 (source) uses shared interface name  "br0".
>> hypervisor2 (target) uses shared interface name "br500".
>> Live migration fails, because target hypervisor does not have "br0" 
>> interface.
> libvirt supports some hook scripts, which can be used to re-write the
> XML file during live migration. Have a look at
> , which
> should work similarly for your scenario.

In the general case of "I need to modify the XML" you can use migration
hooks to modify it, but in the case described in this question you don't
need to modify the XML. Instead, just create an unmanaged libvirt
network on each host, with the same network name, but each pointing to a
different bridge. Then the XML can remain the same.

This same question was also asked simultaneously on libvirt-users, but
in a separate message so the replies for that didn't come to this list.
It looks like the poster used some sort of web interface at gmane.org
which isn't smart enough to use the same message ID when crossposting.
Here is a pointer to my response in libvirt-users:

   https://www.redhat.com/archives/libvirt-users/2015-June/msg00015.html

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] util: Fix coverity warings in virProcessGetAffinity

[Eric Blake]

On 06/10/2015 09:27 AM, John Ferlan wrote:

>> So there are basically three options:
>> 1) Silence the coverity warning
> 
> So on line just prior to CPU_ISSET_S either:
> 
> sa_assert(sizeof(unsigned long int) == sizeof(cpu_set_t));

Is that true even on 32-bit platforms?

> 
> or
> 
> /* coverity[overrun-local] */

Might be safer, even though I generally prefer sa_assert() over /* magic
comment */

-- 
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