[libvirt] [libvirt-glib v5] gobject: Port to GTask API

[Zeeshan Ali (Khattak)]

Drop usage of deprecated GSimpleAsyncResult API.
---

v5:

* Plug some GError leaks.
* Drop StoragePoolBuildData in favour of GUINT_TO_POINTER/GPOINTER_TO_UINT.
* Don't unref GTask before using it later in idle callback.

 libvirt-gobject/libvirt-gobject-domain.c| 290 +--
 libvirt-gobject/libvirt-gobject-input-stream.c  |  78 +++
 libvirt-gobject/libvirt-gobject-output-stream.c |  76 +++---
 libvirt-gobject/libvirt-gobject-storage-pool.c  | 296 +---
 libvirt-gobject/libvirt-gobject-stream.c|  54 +++--
 5 files changed, 320 insertions(+), 474 deletions(-)

diff --git a/libvirt-gobject/libvirt-gobject-domain.c 
b/libvirt-gobject/libvirt-gobject-domain.c
index 5509ce3..c768cd3 100644
--- a/libvirt-gobject/libvirt-gobject-domain.c
+++ b/libvirt-gobject/libvirt-gobject-domain.c
@@ -370,28 +370,20 @@ gboolean gvir_domain_start(GVirDomain *dom,
 return TRUE;
 }
 
-typedef struct {
-guint flags;
-} DomainStartData;
-
-static void domain_start_data_free(DomainStartData *data)
-{
-g_slice_free(DomainStartData, data);
-}
-
 static void
-gvir_domain_start_helper(GSimpleAsyncResult *res,
- GObject *object,
+gvir_domain_start_helper(GTask *task,
+ gpointer source_object,
+ gpointer task_data,
  GCancellable *cancellable G_GNUC_UNUSED)
 {
-GVirDomain *dom = GVIR_DOMAIN(object);
-DomainStartData *data;
+GVirDomain *dom = GVIR_DOMAIN(source_object);
+guint flags = GPOINTER_TO_UINT(task_data);
 GError *err = NULL;
 
-data = g_simple_async_result_get_op_res_gpointer(res);
-
-if (!gvir_domain_start(dom, data->flags, &err))
-g_simple_async_result_take_error(res, err);
+if (!gvir_domain_start(dom, flags, &err))
+g_task_return_error(task, err);
+else
+g_task_return_boolean(task, TRUE);
 }
 
 /**
@@ -410,25 +402,18 @@ void gvir_domain_start_async(GVirDomain *dom,
  GAsyncReadyCallback callback,
  gpointer user_data)
 {
-GSimpleAsyncResult *res;
-DomainStartData *data;
+GTask *task;
 
 g_return_if_fail(GVIR_IS_DOMAIN(dom));
 g_return_if_fail((cancellable == NULL) || G_IS_CANCELLABLE(cancellable));
 
-data = g_slice_new0(DomainStartData);
-data->flags = flags;
-
-res = g_simple_async_result_new(G_OBJECT(dom),
-callback,
-user_data,
-gvir_domain_start_async);
-g_simple_async_result_set_op_res_gpointer (res, data, 
(GDestroyNotify)domain_start_data_free);
-g_simple_async_result_run_in_thread(res,
-gvir_domain_start_helper,
-G_PRIORITY_DEFAULT,
-cancellable);
-g_object_unref(res);
+task = g_task_new(G_OBJECT(dom),
+  cancellable,
+  callback,
+  user_data);
+g_task_set_task_data(task, GUINT_TO_POINTER(flags), NULL);
+g_task_run_in_thread(task, gvir_domain_start_helper);
+g_object_unref(task);
 }
 
 gboolean gvir_domain_start_finish(GVirDomain *dom,
@@ -436,13 +421,10 @@ gboolean gvir_domain_start_finish(GVirDomain *dom,
   GError **err)
 {
 g_return_val_if_fail(GVIR_IS_DOMAIN(dom), FALSE);
-g_return_val_if_fail(g_simple_async_result_is_valid(result, G_OBJECT(dom), 
gvir_domain_start_async), FALSE);
+g_return_val_if_fail(g_task_is_valid(result, dom), FALSE);
 g_return_val_if_fail(err == NULL || *err == NULL, FALSE);
 
-if (g_simple_async_result_propagate_error(G_SIMPLE_ASYNC_RESULT(result), 
err))
-return FALSE;
-
-return TRUE;
+return g_task_propagate_boolean(G_TASK(result), err);
 }
 
 /**
@@ -472,15 +454,18 @@ gboolean gvir_domain_resume(GVirDomain *dom,
 }
 
 static void
-gvir_domain_resume_helper(GSimpleAsyncResult *res,
-  GObject *object,
+gvir_domain_resume_helper(GTask *task,
+  gpointer source_object,
+  gpointer task_data G_GNUC_UNUSED,
   GCancellable *cancellable G_GNUC_UNUSED)
 {
-GVirDomain *dom = GVIR_DOMAIN(object);
+GVirDomain *dom = GVIR_DOMAIN(source_object);
 GError *err = NULL;
 
 if (!gvir_domain_resume(dom, &err))
-g_simple_async_result_take_error(res, err);
+g_task_return_error(task, err);
+else
+g_task_return_boolean(task, TRUE);
 }
 
 /**
@@ -497,20 +482,17 @@ void gvir_domain_resume_async(GVirDomain *dom,
   GAsyncReadyCallback callback,
   gpointer user_data)
 {
-GSimpleAsyncResult *res;
+GTask *task;
 
 g_return_if_fail(GVIR_IS_DOMAIN(dom));
 g_return_if_fail((cancellable == NULL) || G_IS_CANCELLABLE(cancellable

[libvirt] [PATCH 6/6] virsh: Add build flags to pool-create[-as] and pool-start

[John Ferlan]

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

Utilize recently added VIR_STORAGE_POOL_CREATE_WITH_BUILD* flags in
order to pass the flags along to the virStoragePoolCreateXML and
virStoragePoolCreate API's.

This affects the 'virsh pool-create', 'virsh pool-create-as', and
'virsh pool-start' commands.  While it could be argued that pool-start
doesn't need the flags, they could prove useful for someone trying to
do one command build --overwrite and start command processing or
essentially starting with a clean slate.

NB:
This patch is loosely based upon code originally authored by Osier
Yang that were not reviewed and pushed, see:

https://www.redhat.com/archives/libvir-list/2012-July/msg00497.html
Signed-off-by: John Ferlan 
---
 tools/virsh-pool.c | 73 +++---
 tools/virsh.pod| 25 ++-
 2 files changed, 94 insertions(+), 4 deletions(-)

diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index cb91cd3..1bb987d 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -47,6 +47,13 @@
  .help = N_("file containing an XML pool description")\
 },\
 
+#define OPT_BUILD_COMMON  \
+{.name = "build", \
+ .type = VSH_OT_BOOL, \
+ .flags = 0,  \
+ .help = N_("build the pool as normal")   \
+},\
+
 #define OPT_NO_OVERWRITE_COMMON   \
 {.name = "no-overwrite",  \
  .type = VSH_OT_BOOL, \
@@ -235,6 +242,9 @@ static const vshCmdInfo info_pool_create[] = {
 
 static const vshCmdOptDef opts_pool_create[] = {
 OPT_FILE_COMMON
+OPT_BUILD_COMMON
+OPT_NO_OVERWRITE_COMMON
+OPT_OVERWRITE_COMMON
 
 {.name = NULL}
 };
@@ -246,15 +256,32 @@ cmdPoolCreate(vshControl *ctl, const vshCmd *cmd)
 const char *from = NULL;
 bool ret = true;
 char *buffer;
+bool build;
+bool overwrite;
+bool no_overwrite;
+unsigned int flags = 0;
 virshControlPtr priv = ctl->privData;
 
 if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0)
 return false;
 
+build = vshCommandOptBool(cmd, "build");
+overwrite = vshCommandOptBool(cmd, "overwrite");
+no_overwrite = vshCommandOptBool(cmd, "no-overwrite");
+
+VSH_EXCLUSIVE_OPTIONS_VAR(overwrite, no_overwrite);
+
+if (build)
+flags |= VIR_STORAGE_POOL_CREATE_WITH_BUILD;
+if (overwrite)
+flags |= VIR_STORAGE_POOL_CREATE_WITH_BUILD_OVERWRITE;
+if (no_overwrite)
+flags |= VIR_STORAGE_POOL_CREATE_WITH_BUILD_NO_OVERWRITE;
+
 if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0)
 return false;
 
-pool = virStoragePoolCreateXML(priv->conn, buffer, 0);
+pool = virStoragePoolCreateXML(priv->conn, buffer, flags);
 VIR_FREE(buffer);
 
 if (pool != NULL) {
@@ -386,6 +413,9 @@ static const vshCmdInfo info_pool_create_as[] = {
 
 static const vshCmdOptDef opts_pool_create_as[] = {
 OPTS_POOL_COMMON_X_AS
+OPT_BUILD_COMMON
+OPT_NO_OVERWRITE_COMMON
+OPT_OVERWRITE_COMMON
 
 {.name = NULL}
 };
@@ -397,8 +427,25 @@ cmdPoolCreateAs(vshControl *ctl, const vshCmd *cmd)
 const char *name;
 char *xml;
 bool printXML = vshCommandOptBool(cmd, "print-xml");
+bool build;
+bool overwrite;
+bool no_overwrite;
+unsigned int flags = 0;
 virshControlPtr priv = ctl->privData;
 
+build = vshCommandOptBool(cmd, "build");
+overwrite = vshCommandOptBool(cmd, "overwrite");
+no_overwrite = vshCommandOptBool(cmd, "no-overwrite");
+
+VSH_EXCLUSIVE_OPTIONS_VAR(overwrite, no_overwrite);
+
+if (build)
+flags |= VIR_STORAGE_POOL_CREATE_WITH_BUILD;
+if (overwrite)
+flags |= VIR_STORAGE_POOL_CREATE_WITH_BUILD_OVERWRITE;
+if (no_overwrite)
+flags |= VIR_STORAGE_POOL_CREATE_WITH_BUILD_NO_OVERWRITE;
+
 if (!virshBuildPoolXML(ctl, cmd, &name, &xml))
 return false;
 
@@ -406,7 +453,7 @@ cmdPoolCreateAs(vshControl *ctl, const vshCmd *cmd)
 vshPrint(ctl, "%s", xml);
 VIR_FREE(xml);
 } else {
-pool = virStoragePoolCreateXML(priv->conn, xml, 0);
+pool = virStoragePoolCreateXML(priv->conn, xml, flags);
 VIR_FREE(xml);
 
 if (pool != NULL) {
@@ -1657,6 +1704,9 @@ static const vshCmdInfo info_pool_start[] = {
 
 static const vshCmdOptDef opts_pool_start[] = {
 OPT_POOL_COMMON
+OPT_BUILD_COMMON
+OPT_NO_OVERWRITE_COMMON
+OPT_OVERWRITE_COMMON
 
 {.name = NULL}
 };
@@ -1667,11 +1717,28 @@ cmdPoolStart(vshControl *ctl, const vshCmd *cmd)
 virStoragePoolPtr pool;
 bool ret = true;
 const char *name = NULL;
+bool build;
+bool ov

[libvirt] [PATCH 2/6] virsh: Create macro for "pool" option

[John Ferlan]

Rather than continually cut/paste the "pool" option for pool command
option structures, generate a macro which will commonly define it for
any command.  Then of course use that macro.

Signed-off-by: John Ferlan 
---
 tools/virsh-pool.c | 91 +++---
 1 file changed, 31 insertions(+), 60 deletions(-)

diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index cf5a8f3..6922ad5 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -33,6 +33,13 @@
 #include "conf/storage_conf.h"
 #include "virstring.h"
 
+#define OPT_POOL_COMMON   \
+{.name = "pool",  \
+ .type = VSH_OT_DATA, \
+ .flags = VSH_OFLAG_REQ,  \
+ .help = N_("pool name or uuid")  \
+},\
+
 virStoragePoolPtr
 virshCommandOptPoolBy(vshControl *ctl, const vshCmd *cmd, const char *optname,
   const char **name, unsigned int flags)
@@ -85,11 +92,8 @@ static const vshCmdInfo info_pool_autostart[] = {
 };
 
 static const vshCmdOptDef opts_pool_autostart[] = {
-{.name = "pool",
- .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
- .help = N_("pool name or uuid")
-},
+OPT_POOL_COMMON
+
 {.name = "disable",
  .type = VSH_OT_BOOL,
  .help = N_("disable autostarting")
@@ -500,11 +504,8 @@ static const vshCmdInfo info_pool_build[] = {
 };
 
 static const vshCmdOptDef opts_pool_build[] = {
-{.name = "pool",
- .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
- .help = N_("pool name or uuid")
-},
+OPT_POOL_COMMON
+
 {.name = "no-overwrite",
  .type = VSH_OT_BOOL,
  .help = N_("do not overwrite an existing pool of this type")
@@ -559,11 +560,8 @@ static const vshCmdInfo info_pool_destroy[] = {
 };
 
 static const vshCmdOptDef opts_pool_destroy[] = {
-{.name = "pool",
- .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
- .help = N_("pool name or uuid")
-},
+OPT_POOL_COMMON
+
 {.name = NULL}
 };
 
@@ -602,11 +600,8 @@ static const vshCmdInfo info_pool_delete[] = {
 };
 
 static const vshCmdOptDef opts_pool_delete[] = {
-{.name = "pool",
- .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
- .help = N_("pool name or uuid")
-},
+OPT_POOL_COMMON
+
 {.name = NULL}
 };
 
@@ -645,11 +640,8 @@ static const vshCmdInfo info_pool_refresh[] = {
 };
 
 static const vshCmdOptDef opts_pool_refresh[] = {
-{.name = "pool",
- .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
- .help = N_("pool name or uuid")
-},
+OPT_POOL_COMMON
+
 {.name = NULL}
 };
 
@@ -688,11 +680,8 @@ static const vshCmdInfo info_pool_dumpxml[] = {
 };
 
 static const vshCmdOptDef opts_pool_dumpxml[] = {
-{.name = "pool",
- .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
- .help = N_("pool name or uuid")
-},
+OPT_POOL_COMMON
+
 {.name = "inactive",
  .type = VSH_OT_BOOL,
  .help = N_("show inactive defined XML")
@@ -1542,11 +1531,8 @@ static const vshCmdInfo info_pool_info[] = {
 };
 
 static const vshCmdOptDef opts_pool_info[] = {
-{.name = "pool",
- .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
- .help = N_("pool name or uuid")
-},
+OPT_POOL_COMMON
+
 {.name = NULL}
 };
 
@@ -1622,11 +1608,8 @@ static const vshCmdInfo info_pool_name[] = {
 };
 
 static const vshCmdOptDef opts_pool_name[] = {
-{.name = "pool",
- .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
- .help = N_("pool uuid")
-},
+OPT_POOL_COMMON
+
 {.name = NULL}
 };
 
@@ -1657,11 +1640,8 @@ static const vshCmdInfo info_pool_start[] = {
 };
 
 static const vshCmdOptDef opts_pool_start[] = {
-{.name = "pool",
- .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
- .help = N_("name or uuid of the inactive pool")
-},
+OPT_POOL_COMMON
+
 {.name = NULL}
 };
 
@@ -1700,11 +1680,8 @@ static const vshCmdInfo info_pool_undefine[] = {
 };
 
 static const vshCmdOptDef opts_pool_undefine[] = {
-{.name = "pool",
- .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
- .help = N_("pool name or uuid")
-},
+OPT_POOL_COMMON
+
 {.name = NULL}
 };
 
@@ -1743,11 +1720,8 @@ static const vshCmdInfo info_pool_uuid[] = {
 };
 
 static const vshCmdOptDef opts_pool_uuid[] = {
-{.name = "pool",
- .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
- .help = N_("pool name")
-},
+OPT_POOL_COMMON
+
 {.name = NULL}
 };
 
@@ -1783,11 +1757,8 @@ static const vshCmdInfo info_pool_edit[] = {
 };
 
 static const vshCmdOptDef opts_pool_edit[] = {
-{.name = "pool",
- .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
- .help = N_("pool name or uuid")
-},
+OPT_POOL_COMMON
+
 {.name = NULL}
 };
 
-- 
2.5.0

--
libvir-list mailing list
libvir-list@redhat.com
https

[libvirt] [PATCH 1/6] storage: Add flags to allow building pool during create processing

[John Ferlan]

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

Add flags handling to the virStoragePoolCreate and virStoragePoolCreateXML
API's which will allow the caller to provide the capability for the storage
pool create API's to also perform a pool build during creation rather than
requiring the additional buildPool step. This will allow transient pools
to be defined, built, and started.

The new flags are:

* VIR_STORAGE_POOL_CREATE_WITH_BUILD
  Perform buildPool without any flags passed.

* VIR_STORAGE_POOL_CREATE_WITH_BUILD_OVERWRITE
  Perform buildPool using VIR_STORAGE_POOL_BUILD_OVERWRITE flag.

* VIR_STORAGE_POOL_CREATE_WITH_BUILD_NO_OVERWRITE
  Perform buildPool using VIR_STORAGE_POOL_BUILD_NO_OVERWRITE flag.

It is up to the backend to handle the processing of build flags. The
overwrite and no-overwrite flags are mutually exclusive.

NB:
This patch is loosely based upon code originally authored by Osier
Yang that were not reviewed and pushed, see:

https://www.redhat.com/archives/libvir-list/2012-July/msg01328.html
Signed-off-by: John Ferlan 
---
 include/libvirt/libvirt-storage.h | 16 ++-
 src/libvirt-storage.c |  4 ++--
 src/storage/storage_driver.c  | 42 +--
 3 files changed, 57 insertions(+), 5 deletions(-)

diff --git a/include/libvirt/libvirt-storage.h 
b/include/libvirt/libvirt-storage.h
index 9fc3c2d..996a726 100644
--- a/include/libvirt/libvirt-storage.h
+++ b/include/libvirt/libvirt-storage.h
@@ -57,7 +57,6 @@ typedef enum {
 # endif
 } virStoragePoolState;
 
-
 typedef enum {
 VIR_STORAGE_POOL_BUILD_NEW  = 0,   /* Regular build from scratch */
 VIR_STORAGE_POOL_BUILD_REPAIR = (1 << 0), /* Repair / reinitialize */
@@ -71,6 +70,21 @@ typedef enum {
 VIR_STORAGE_POOL_DELETE_ZEROED = 1 << 0,  /* Clear all data to zeros 
(slow) */
 } virStoragePoolDeleteFlags;
 
+typedef enum {
+VIR_STORAGE_POOL_CREATE_NORMAL = 0,
+
+/* Create the pool and perform pool build without any flags */
+VIR_STORAGE_POOL_CREATE_WITH_BUILD = 1 << 0,
+
+/* Create the pool and perform pool build using the
+ * VIR_STORAGE_POOL_BUILD_OVERWRITE flag */
+VIR_STORAGE_POOL_CREATE_WITH_BUILD_OVERWRITE = 1 << 1,
+
+/* Create the pool and perform pool build using the
+ * VIR_STORAGE_POOL_BUILD_NO_OVERWRITE flag */
+VIR_STORAGE_POOL_CREATE_WITH_BUILD_NO_OVERWRITE = 1 << 2,
+} virStoragePoolCreateFlags;
+
 typedef struct _virStoragePoolInfo virStoragePoolInfo;
 
 struct _virStoragePoolInfo {
diff --git a/src/libvirt-storage.c b/src/libvirt-storage.c
index 66dd9f0..238a6cd 100644
--- a/src/libvirt-storage.c
+++ b/src/libvirt-storage.c
@@ -505,7 +505,7 @@ virStoragePoolLookupByVolume(virStorageVolPtr vol)
  * virStoragePoolCreateXML:
  * @conn: pointer to hypervisor connection
  * @xmlDesc: XML description for new pool
- * @flags: extra flags; not used yet, so callers should always pass 0
+ * @flags: bitwise-OR of virStoragePoolCreateFlags
  *
  * Create a new storage based on its XML description. The
  * pool is not persistent, so its definition will disappear
@@ -670,7 +670,7 @@ virStoragePoolUndefine(virStoragePoolPtr pool)
 /**
  * virStoragePoolCreate:
  * @pool: pointer to storage pool
- * @flags: extra flags; not used yet, so callers should always pass 0
+ * @flags: bitwise-OR of virStoragePoolCreateFlags
  *
  * Starts an inactive storage pool
  *
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index bbf21f6..59a18bf 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -671,8 +671,11 @@ storagePoolCreateXML(virConnectPtr conn,
 virStoragePoolPtr ret = NULL;
 virStorageBackendPtr backend;
 char *stateFile = NULL;
+unsigned int build_flags = 0;
 
-virCheckFlags(0, NULL);
+virCheckFlags(VIR_STORAGE_POOL_CREATE_WITH_BUILD |
+  VIR_STORAGE_POOL_CREATE_WITH_BUILD_OVERWRITE |
+  VIR_STORAGE_POOL_CREATE_WITH_BUILD_NO_OVERWRITE, NULL);
 
 storageDriverLock();
 if (!(def = virStoragePoolDefParseString(xml)))
@@ -694,6 +697,22 @@ storagePoolCreateXML(virConnectPtr conn,
 goto cleanup;
 def = NULL;
 
+if (backend->buildPool) {
+if (flags & VIR_STORAGE_POOL_CREATE_WITH_BUILD_OVERWRITE)
+build_flags |= VIR_STORAGE_POOL_BUILD_OVERWRITE;
+else if (flags & VIR_STORAGE_POOL_CREATE_WITH_BUILD_NO_OVERWRITE)
+build_flags |= VIR_STORAGE_POOL_BUILD_NO_OVERWRITE;
+
+if (build_flags ||
+(flags & VIR_STORAGE_POOL_CREATE_WITH_BUILD)) {
+if (backend->buildPool(conn, pool, build_flags) < 0) {
+virStoragePoolObjRemove(&driver->pools, pool);
+pool = NULL;
+goto cleanup;
+}
+}
+}
+
 if (backend->startPool &&
 backend->startPool(conn, pool) < 0) {
 virStoragePoolObjRemove(&driver->pools, pool);
@@ -845,8 +864,11 @@ storagePoolCreate(virS

[libvirt] [PATCH 0/6] Add build options to pool create[-as] and start

[John Ferlan]

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

Add the capability to build the pool during pool creation or startup

NOTE: If patches 2-4 regarding usage of #define for repeated command
options are acceptible, I can start doing the same for other virsh
commands. I just found it easier and I think better than the need to
continually cut-n-paste for each command option entry. If it's preferred
to keep cut-n-paste'ing, then I can modify patch 5 & 6 to follow that model.

NOTE: Based loosely on patches posted quite a while ago by Osier Yang
before he left Red Hat, see:

(Cover)
https://www.redhat.com/archives/libvir-list/2012-July/msg00494.html

and

(libvirt API, storage driver changes)
https://www.redhat.com/archives/libvir-list/2012-July/msg01328.html

and

(virsh changes)
https://www.redhat.com/archives/libvir-list/2012-July/msg00497.html

John Ferlan (6):
  storage: Add flags to allow building pool during create processing
  virsh: Create macro for "pool" option
  virsh: Create macro for "file" option
  virsh: Create macro for "overwrite" and no-overwrite" options
  virsh: Create a macro for pool-define-as and pool-create-as options
  virsh: Add build flags to pool-create[-as] and pool-start

 include/libvirt/libvirt-storage.h |  16 +-
 src/libvirt-storage.c |   4 +-
 src/storage/storage_driver.c  |  42 -
 tools/virsh-pool.c| 360 ++
 tools/virsh.pod   |  25 ++-
 5 files changed, 288 insertions(+), 159 deletions(-)

-- 
2.5.0

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

[libvirt] [PATCH 4/6] virsh: Create macro for "overwrite" and no-overwrite" options

[John Ferlan]

Although not currently used in more than one command, it soon will be
so create a common macro to be used in the new command location.

Additionally, add the ".flags = 0," for both to match the expections
of the structure being predefined.

Signed-off-by: John Ferlan 
---
 tools/virsh-pool.c | 24 
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index 4b4fea0..ff77039 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -47,6 +47,20 @@
  .help = N_("file containing an XML pool description")\
 },\
 
+#define OPT_NO_OVERWRITE_COMMON   \
+{.name = "no-overwrite",  \
+ .type = VSH_OT_BOOL, \
+ .flags = 0,  \
+ .help = N_("do not overwrite an existing pool of this type") \
+},\
+
+#define OPT_OVERWRITE_COMMON  \
+{.name = "overwrite", \
+ .type = VSH_OT_BOOL, \
+ .flags = 0,  \
+ .help = N_("overwrite any existing data")\
+},\
+
 virStoragePoolPtr
 virshCommandOptPoolBy(vshControl *ctl, const vshCmd *cmd, const char *optname,
   const char **name, unsigned int flags)
@@ -506,15 +520,9 @@ static const vshCmdInfo info_pool_build[] = {
 
 static const vshCmdOptDef opts_pool_build[] = {
 OPT_POOL_COMMON
+OPT_NO_OVERWRITE_COMMON
+OPT_OVERWRITE_COMMON
 
-{.name = "no-overwrite",
- .type = VSH_OT_BOOL,
- .help = N_("do not overwrite an existing pool of this type")
-},
-{.name = "overwrite",
- .type = VSH_OT_BOOL,
- .help = N_("overwrite any existing data")
-},
 {.name = NULL}
 };
 
-- 
2.5.0

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

[libvirt] [PATCH 3/6] virsh: Create macro for "file" option

[John Ferlan]

Rather than continually cut/paste the "file" option for pool command
option structures, generate a macro which will commonly define it for
any command.  Then of course use that macro.

Signed-off-by: John Ferlan 
---
 tools/virsh-pool.c | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index 6922ad5..4b4fea0 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -40,6 +40,13 @@
  .help = N_("pool name or uuid")  \
 },\
 
+#define OPT_FILE_COMMON   \
+{.name = "file",  \
+ .type = VSH_OT_DATA, \
+ .flags = VSH_OFLAG_REQ,  \
+ .help = N_("file containing an XML pool description")\
+},\
+
 virStoragePoolPtr
 virshCommandOptPoolBy(vshControl *ctl, const vshCmd *cmd, const char *optname,
   const char **name, unsigned int flags)
@@ -145,11 +152,8 @@ static const vshCmdInfo info_pool_create[] = {
 };
 
 static const vshCmdOptDef opts_pool_create[] = {
-{.name = "file",
- .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
- .help = N_("file containing an XML pool description")
-},
+OPT_FILE_COMMON
+
 {.name = NULL}
 };
 
@@ -410,11 +414,8 @@ static const vshCmdInfo info_pool_define[] = {
 };
 
 static const vshCmdOptDef opts_pool_define[] = {
-{.name = "file",
- .type = VSH_OT_DATA,
- .flags = VSH_OFLAG_REQ,
- .help = N_("file containing an XML pool description")
-},
+OPT_FILE_COMMON
+
 {.name = NULL}
 };
 
-- 
2.5.0

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

[libvirt] [PATCH 5/6] virsh: Create a macro for pool-define-as and pool-create-as options

[John Ferlan]

Although they both are the same now, a future patch will add new options
to pool-create-as. So create a common macro to capture commonality, then
use that in the command specific structure.

Signed-off-by: John Ferlan 
---
 tools/virsh-pool.c | 151 -
 1 file changed, 79 insertions(+), 72 deletions(-)

diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index ff77039..cb91cd3 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -61,6 +61,74 @@
  .help = N_("overwrite any existing data")\
 },\
 
+#define OPTS_POOL_COMMON_X_AS  \
+{.name = "name",   \
+ .type = VSH_OT_DATA,  \
+ .flags = VSH_OFLAG_REQ,   \
+ .help = N_("name of the pool")\
+}, \
+{.name = "type",   \
+ .type = VSH_OT_DATA,  \
+ .flags = VSH_OFLAG_REQ,   \
+ .help = N_("type of the pool")\
+}, \
+{.name = "print-xml",  \
+ .type = VSH_OT_BOOL,  \
+ .help = N_("print XML document, but don't define/create") \
+}, \
+{.name = "source-host",\
+ .type = VSH_OT_STRING,\
+ .help = N_("source-host for underlying storage")  \
+}, \
+{.name = "source-path",\
+ .type = VSH_OT_STRING,\
+ .help = N_("source path for underlying storage")  \
+}, \
+{.name = "source-dev", \
+ .type = VSH_OT_STRING,\
+ .help = N_("source device for underlying storage")\
+}, \
+{.name = "source-name",\
+ .type = VSH_OT_STRING,\
+ .help = N_("source name for underlying storage")  \
+}, \
+{.name = "target", \
+ .type = VSH_OT_STRING,\
+ .help = N_("target for underlying storage")   \
+}, \
+{.name = "source-format",  \
+ .type = VSH_OT_STRING,\
+ .help = N_("format for underlying storage")   \
+}, \
+{.name = "auth-type",  \
+ .type = VSH_OT_STRING,\
+ .help = N_("auth type to be used for underlying storage") \
+}, \
+{.name = "auth-username",  \
+ .type = VSH_OT_STRING,\
+ .help = N_("auth username to be used for underlying storage") \
+}, \
+{.name = "secret-usage",   \
+ .type = VSH_OT_STRING,\
+ .help = N_("auth secret usage to be used for underlying storage") \
+}, \
+{.name = "adapter-name",   \
+ .type = VSH_OT_STRING,\
+ .help = N_("adapter name to be used for underlying storage")  \
+}, \
+{.name = "adapter-wwnn",   \
+ .type = VSH_OT_STRING,\
+ .help = N_("adapter wwnn to be used for underlying storage")  \
+}, 

Re: [libvirt] [PATCH] virtlockd: fix misc memory leaks and other bugs

[John Ferlan]

On 11/24/2015 08:17 AM, Daniel P. Berrange wrote:
> Fix memory leaks, failure to restore umask and missing man
> page docs.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  src/locking/lock_daemon.c| 6 +-
>  src/locking/virtlockd.pod.in | 7 ++-
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 

ACK,

John

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

Re: [libvirt] [PATCH] logging: inhibit virtlogd shutdown while log files are open

[John Ferlan]

On 11/24/2015 08:10 AM, Daniel P. Berrange wrote:
> The virtlogd daemon is launched with a 30 second timeout for
> unprivileged users. Unfortunately the timeout is only inhibited
> while RPC clients are connected, and they only connect for a
> short while to open the log file descriptor. We need to hold
> an inhibition for as long as the log file descriptor itself
> is open.
> 
> Signed-off-by: Daniel P. Berrange 
> ---
>  src/logging/log_daemon.c  | 19 +--
>  src/logging/log_handler.c | 33 +++--
>  src/logging/log_handler.h | 11 +--
>  3 files changed, 53 insertions(+), 10 deletions(-)
> 

ACK

John

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

Re: [libvirt] [virt-tools-list] ANNOUNCE: virt-manager 1.3.0 released

[Kevin Zhao]

Congrats !
And Big Thanks Cole ~

On 2015年11月25日 11:11, Cole Robinson wrote:

I'm happy to announce the release of virt-manager 1.3.0!

virt-manager is a desktop application for managing KVM, Xen, and LXC
virtualization via libvirt.

The release can be downloaded from:

http://virt-manager.org/download/

This release includes:

- Git hosting moved to http://github.com/virt-manager/virt-manager
- Switch translation infrastructure from transifex to fedora.zanata.org
- Add dogtail UI tests and infrastructure
- Improved support for s390x kvm (Kevin Zhao)
- virt-install and virt-manager now remove created disk images if VM
   install startup fails
- Replace urlgrabber usage with requests and urllib2
- virt-install: add --network virtualport support for openvswitch
   (Daniel P. Berrange)
- virt-install: support multiple --security labels
- virt-install: support --features kvm_hidden=on|off (Pavel Hrdina)
- virt-install: add --features pmu=on|off
- virt-install: add --features pvspinlock=on|off (Abhijeet Kasurde)
- virt-install: add --events on_lockfailure=on|off (Abhijeet Kasurde)
- virt-install: add --network link_state=up|down
- virt-install: add --vcpu placement=static|auto

Thanks to everyone who has contributed to this release through testing,
bug reporting, submitting patches, and otherwise sending in feedback!

Thanks,
Cole

___
virt-tools-list mailing list
virt-tools-l...@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list



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

Re: [libvirt] [PATCH v3 02/11] virt-admin: Introduce first working skeleton

[Erik Skultety]

>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library.  If not, see
>> + * .
>> + *
>> + * Erik Skultety 
> 
> We usually prepend this line with "Authors: ".
> 

How can I keep forgetting these things?! I'll fix it, thanks.

>> +static bool
>> +cmdConnect(vshControl *ctl, const vshCmd *cmd)
>> +{
>> +const char *name = NULL;
>> +vshAdmControlPtr priv = ctl->privData;
>> +
>> +VIR_FREE(ctl->connname);
>> +if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0)
>> +return false;
> 
> I think the VIR_FREE() should go after option acquiring. Otherwise we
> will lose it if vshCommand... fails.
> 

Spurious as it might be, this could only fail, if we decided that --name
argument to virsh/virt-admin connect command should be required, but
since it's optional, vshCommandOptStringReq can never fail in this
specific case (I guess I already mentioned this in one of my earlier
replies to previous versions). But I do agree with you that it would be
nice to fix the logic, so no future questions about this snippet of code
will arise.

>> +ctl->connname = vshStrdup(ctl, name);
>> +
>> +vshAdmReconnect(ctl);
>> +
>> +return !!priv->conn;
>> +}
> 
> ACK
> 
> Michal
> 

Thanks, I'll adjust the patch and push it once the series is complete.

Erik

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

[libvirt] [PATCH] Enhance documentation of virDomainDetachDevice

[Jiri Denemark]

Link it to virDomainDetachDeviceFlags.

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

Signed-off-by: Jiri Denemark 
---
 src/libvirt-domain.c | 15 +++
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 7cfbe58..7cfc4d2 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -8392,19 +8392,10 @@ virDomainAttachDeviceFlags(virDomainPtr domain,
  * @domain: pointer to domain object
  * @xml: pointer to XML description of one device
  *
- * Destroy a virtual device attachment to backend.  This function,
- * having hot-unplug semantics, is only allowed on an active domain.
+ * This is an equivalent of virDomainDetachDeviceFlags() when called with
+ * @flags parameter set to VIR_DOMAIN_AFFECT_LIVE.
  *
- * Be aware that hotplug changes might not persist across a domain going
- * into S4 state (also known as hibernation) unless you also modify the
- * persistent domain definition.
- *
- * The supplied XML description of the device should be as specific
- * as its definition in the domain XML. The set of attributes used
- * to match the device are internal to the drivers. Using a partial definition,
- * or attempting to detach a device that is not present in the domain XML,
- * but shares some specific attributes with one that is present,
- * may lead to unexpected results.
+ * See virDomainDetachDeviceFlags() for more details.
  *
  * Returns 0 in case of success, -1 in case of failure.
  */
-- 
2.6.3

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

Re: [libvirt] [PATCH] lxc: drop sys_admin caps by default

[Daniel P. Berrange]

On Wed, Nov 25, 2015 at 03:40:36PM +0100, Cédric Bosdonnat wrote:
> To make sure the container user doesn't play with mounts, like
> changing them from ro to rw, drop the sys_admin capability by default.
> If user really needs to play with those, it can be enabled in the
> configuration.
> ---
>  Note: it seems that patch 3/3 or my last series never reached the list.
>  Here it is.
> 
>  src/lxc/lxc_container.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
> index c5a70a1..d6d6fba 100644
> --- a/src/lxc/lxc_container.c
> +++ b/src/lxc/lxc_container.c
> @@ -2083,6 +2083,7 @@ static int lxcContainerDropCapabilities(virDomainDefPtr 
> def,
>  case VIR_DOMAIN_CAPS_FEATURE_MKNOD: /* No creating device nodes 
> */
>  case VIR_DOMAIN_CAPS_FEATURE_AUDIT_CONTROL: /* No messing with 
> auditing status */
>  case VIR_DOMAIN_CAPS_FEATURE_MAC_ADMIN: /* No messing with LSM 
> config */
> +case VIR_DOMAIN_CAPS_FEATURE_SYS_ADMIN: /* No messing with 
> mounts */
>  toDrop = (state != VIR_TRISTATE_SWITCH_ON);
>  break;
>  default: /* User specified capabilities to drop */

I don't think we really need/want this.

If usernamespace is enabled, it is perfectly safe to have CAP_SYS_ADMIN.

If usernamespace is disabled, then whether or not you have CAP_SYS_ADMIN
is not significant - you need to use SELinux/AppArmour to provide any
kind of protection.

For those existing feature flags we just disable them by default for
historical reasons, and I don't think we should add more to them.
If it weren't for historical practice, we'd just leave all capabilities
enabled all the time.

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 v3 02/11] virt-admin: Introduce first working skeleton

[Michal Privoznik]

On 06.11.2015 12:46, Erik Skultety wrote:
> This patch introduces virt-admin client which is based on virsh client,
> but had to reimplement several methods to meet virt-admin specific needs
> or remove unnecessary virsh specific logic.
> ---
>  .gitignore |   1 +
>  po/POTFILES.in |   1 +
>  tools/Makefile.am  |  26 ++-
>  tools/virt-admin.c | 556 
> +
>  tools/virt-admin.h |  46 +
>  5 files changed, 628 insertions(+), 2 deletions(-)
>  create mode 100644 tools/virt-admin.c
>  create mode 100644 tools/virt-admin.h
> 

> diff --git a/tools/virt-admin.c b/tools/virt-admin.c
> new file mode 100644
> index 000..ddfba91
> --- /dev/null
> +++ b/tools/virt-admin.c
> @@ -0,0 +1,556 @@
> +/*
> + * virt-admin.c: a shell to exercise the libvirt admin API
> + *
> + * Copyright (C) 2015 Red Hat, Inc.
> + *
> + * 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, see
> + * .
> + *
> + * Erik Skultety 

We usually prepend this line with "Authors: ".

> + */
> +
> +#include 
> +#include "virt-admin.h"
> +
> +#include 
> +#include 
> +#include 
> +
> +#if WITH_READLINE
> +# include 
> +# include 
> +#endif
> +
> +#include "configmake.h"
> +#include "internal.h"
> +#include "viralloc.h"
> +#include "virerror.h"
> +#include "virfile.h"
> +#include "virstring.h"
> +#include "virthread.h"
> +
> +/* Gnulib doesn't guarantee SA_SIGINFO support.  */
> +#ifndef SA_SIGINFO
> +# define SA_SIGINFO 0
> +#endif
> +
> +#define VIRT_ADMIN_PROMPT "virt-admin # "
> +
> +static char *progname;
> +
> +static const vshCmdGrp cmdGroups[];
> +static const vshClientHooks hooks;
> +
> +static int
> +vshAdmConnect(vshControl *ctl, unsigned int flags)
> +{
> +vshAdmControlPtr priv = ctl->privData;
> +
> +priv->conn = virAdmConnectOpen(ctl->connname, flags);
> +
> +if (!priv->conn) {
> +if (priv->wantReconnect)
> +vshError(ctl, "%s", _("Failed to reconnect to the admin 
> server"));
> +else
> +vshError(ctl, "%s", _("Failed to connect to the admin server"));
> +return -1;
> +} else {
> +if (priv->wantReconnect)
> +vshPrint(ctl, "%s\n", _("Reconnected to the admin server"));
> +else
> +vshPrint(ctl, "%s\n", _("Connected to the admin server"));
> +}
> +
> +return 0;
> +}
> +
> +static int
> +vshAdmDisconnect(vshControl *ctl)
> +{
> +int ret = 0;
> +vshAdmControlPtr priv = ctl->privData;
> +
> +if (!priv->conn)
> +return ret;
> +
> +ret = virAdmConnectClose(priv->conn);
> +if (ret < 0)
> +vshError(ctl, "%s", _("Failed to disconnect from the admin server"));
> +else if (ret > 0)
> +vshError(ctl, "%s", _("One or more references were leaked after "
> +  "disconnect from the hypervisor"));
> +priv->conn = NULL;
> +return ret;
> +}
> +
> +/*
> + * vshAdmReconnect:
> + *
> + * Reconnect to a daemon's admin server
> + *
> + */
> +static void
> +vshAdmReconnect(vshControl *ctl)
> +{
> +vshAdmControlPtr priv = ctl->privData;
> +if (priv->conn)
> +priv->wantReconnect = true;
> +
> +vshAdmDisconnect(ctl);
> +vshAdmConnect(ctl, 0);
> +
> +priv->wantReconnect = false;
> +}
> +
> +/* ---
> + * Command Connect
> + * ---
> + */
> +
> +static const vshCmdOptDef opts_connect[] = {
> +{.name = "name",
> + .type = VSH_OT_STRING,
> + .flags = VSH_OFLAG_EMPTY_OK,
> + .help = N_("daemon's admin server connection URI")
> +},
> +{.name = NULL}
> +};
> +
> +static const vshCmdInfo info_connect[] = {
> +{.name = "help",
> + .data = N_("connect to daemon's admin server")
> +},
> +{.name = "desc",
> + .data = N_("Connect to a daemon's administrating server.")
> +},
> +{.name = NULL}
> +};
> +
> +static bool
> +cmdConnect(vshControl *ctl, const vshCmd *cmd)
> +{
> +const char *name = NULL;
> +vshAdmControlPtr priv = ctl->privData;
> +
> +VIR_FREE(ctl->connname);
> +if (vshCommandOptStringReq(ctl, cmd, "name", &name) < 0)
> +return false;

I think the VIR_FREE() should go after option acquiring. Otherwise we
will lose it if vshCommand... fails.

> +
> +ctl->connname = vshStrdup(ctl, name);
> +
> +vshAdmReconnect(ctl);
> +
> +return !!priv->conn;
> +}

Re: [libvirt] [PATCH v3 11/11] virt-admin: Provide a man page for virt-admin

[Michal Privoznik]

On 06.11.2015 12:46, Erik Skultety wrote:
> ---
>  tools/Makefile.am|  12 +--
>  tools/virt-admin.pod | 255 
> +++
>  2 files changed, 259 insertions(+), 8 deletions(-)
>  create mode 100644 tools/virt-admin.pod
> 
> diff --git a/tools/Makefile.am b/tools/Makefile.am
> index 62073f9..9180564 100644
> --- a/tools/Makefile.am
> +++ b/tools/Makefile.am
> @@ -83,7 +83,8 @@ dist_man1_MANS = \
>   virt-host-validate.1 \
>   virt-pki-validate.1 \
>   virt-xml-validate.1 \
> - virsh.1
> + virsh.1 \
> + virt-admin.1
>  if WITH_LXC
>  dist_man1_MANS += virt-login-shell.1
>  else ! WITH_LXC
> @@ -280,14 +281,9 @@ virsh_win_icon.$(OBJEXT): virsh_win_icon.rc
> --output-format coff --output $@
>  endif WITH_WIN_ICON
>  
> -virt-login-shell.1: virt-login-shell.pod $(top_srcdir)/configure.ac
> +%.1: %.pod $(top_srcdir)/configure.ac
>   $(AM_V_GEN)$(POD2MAN) $< $(srcdir)/$@ \
> - && if grep 'POD ERROR' $(srcdir)/$@ ; then \
> - rm $(srcdir)/$@; exit 1; fi
> -
> -virsh.1: virsh.pod $(top_srcdir)/configure.ac
> - $(AM_V_GEN)$(POD2MAN) $< $(srcdir)/$@ \
> - && if grep 'POD ERROR' $(srcdir)/$@ ; then \
> + && if grep 'POD ERROR' $(srcdir)/$@ ; then \
>   rm $(srcdir)/$@; exit 1; fi
>  
>  install-data-local: install-init install-systemd
> diff --git a/tools/virt-admin.pod b/tools/virt-admin.pod
> new file mode 100644
> index 000..dc8c083
> --- /dev/null
> +++ b/tools/virt-admin.pod
> @@ -0,0 +1,255 @@
> +=head1 NAME
> +
> +virt-admin - daemon administration interface
> +
> +=head1 SYNOPSIS
> +
> +B [I]... [I]
> +
> +B [I]... I [I]...
> +
> +=head1 DESCRIPTION
> +
> +The B program is the main administration interface for modifying
> +the libvirt daemon configuration in runtime, changing daemon behaviour as 
> well

s/in/at/

> +as for monitoring and managing all clients connected to the daemon.
> +
> +The basic structure of most virt-admin usage is:
> +
> +  virt-admin [OPTION]...  [ARG]...
> +
> +Where I is one of the commands listed below.

Otherwise looking good. ACK.

Michal

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

Re: [libvirt] [libvirt-glib v4] gobject: Port to GTask API

[Christophe Fergeau]

Hey,

On Tue, Nov 24, 2015 at 04:23:12PM +, Zeeshan Ali (Khattak) wrote:
> Drop usage of deprecated GSimpleAsyncResult API.
> ---
>  libvirt-gobject/libvirt-gobject-domain.c| 290 
> +---
>  libvirt-gobject/libvirt-gobject-input-stream.c  |  77 +++
>  libvirt-gobject/libvirt-gobject-output-stream.c |  75 +++---
>  libvirt-gobject/libvirt-gobject-storage-pool.c  | 281 ++-
>  libvirt-gobject/libvirt-gobject-stream.c|  53 +++--
>  5 files changed, 322 insertions(+), 454 deletions(-)
> 
> diff --git a/libvirt-gobject/libvirt-gobject-domain.c 
> b/libvirt-gobject/libvirt-gobject-domain.c
> index 5509ce3..c768cd3 100644
> --- a/libvirt-gobject/libvirt-gobject-domain.c
> +++ b/libvirt-gobject/libvirt-gobject-domain.c
> diff --git a/libvirt-gobject/libvirt-gobject-input-stream.c 
> b/libvirt-gobject/libvirt-gobject-input-stream.c
> index ff1a70c..cd107e1 100644
> --- a/libvirt-gobject/libvirt-gobject-input-stream.c
> +++ b/libvirt-gobject/libvirt-gobject-input-stream.c
> @@ -45,8 +45,7 @@ struct _GVirInputStreamPrivate
>  GVirStream *stream;
>  
>  /* pending operation metadata */
> -GSimpleAsyncResult *result;
> -GCancellable *cancellable;
> +GTask *task;
>  gpointer buffer;
>  gsize count;
>  };
> @@ -103,48 +102,44 @@ gvir_input_stream_read_ready(GVirStream *stream,
>  {
>  GVirInputStream *input_stream = GVIR_INPUT_STREAM(opaque);
>  GVirInputStreamPrivate *priv = input_stream->priv;
> -GSimpleAsyncResult *simple = priv->result;
> +GTask *task = priv->task;
> +GCancellable *cancellable = g_task_get_cancellable(task);
>  GError *error = NULL;
>  gssize result;
>  
> +priv->task = NULL;
> +
>  if (!(cond & GVIR_STREAM_IO_CONDITION_READABLE)) {
>  g_warn_if_reached();
> -g_simple_async_result_set_error(simple,
> -G_IO_ERROR,
> -G_IO_ERROR_INVALID_ARGUMENT,
> -"%s",
> -"Expected stream to be readable");
> +g_task_return_new_error(task,
> +G_IO_ERROR,
> +G_IO_ERROR_INVALID_ARGUMENT,
> +"%s",
> +"Expected stream to be readable");
>  goto cleanup;
>  }
>  
> -result  = gvir_stream_receive(stream, priv->buffer, priv->count,
> -  priv->cancellable, &error);
> +result = gvir_stream_receive(stream, priv->buffer, priv->count,
> + cancellable, &error);
> +if (error != NULL) {
> +if (g_error_matches(error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK)) {
> +g_warn_if_reached();
> +g_task_return_new_error(task,
> +G_IO_ERROR,
> +G_IO_ERROR_INVALID_ARGUMENT,
> +"%s",
> +"Expected stream to be readable");

This is preexisting, but I believe 'error' is going to be leaked here.

> +} else {
> +g_task_return_error(task, error);
> +}
>  
> -if (g_error_matches(error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK)) {
> -g_warn_if_reached();
> -g_simple_async_result_set_error(simple,
> -G_IO_ERROR,
> -G_IO_ERROR_INVALID_ARGUMENT,
> -"%s",
> -"Expected stream to be readable");
>  goto cleanup;
>  }
>  
> -if (result >= 0)
> -g_simple_async_result_set_op_res_gssize(simple, result);
> -
> -if (error)
> -g_simple_async_result_take_error(simple, error);
> -
> -if (priv->cancellable) {
> -g_object_unref(priv->cancellable);
> -priv->cancellable = NULL;
> -}
> +g_task_return_int(task, result);
>  
>  cleanup:
> -priv->result = NULL;
> -g_simple_async_result_complete(simple);
> -g_object_unref(simple);
> +g_object_unref(task);
>  return FALSE;
>  }
>  
> @@ -159,7 +154,7 @@ static void gvir_input_stream_read_async(GInputStream 
> *stream,
>  GVirInputStream *input_stream = GVIR_INPUT_STREAM(stream);
>  
>  g_return_if_fail(GVIR_IS_INPUT_STREAM(stream));
> -g_return_if_fail(input_stream->priv->result == NULL);
> +g_return_if_fail(input_stream->priv->task == NULL);
>  
>  gvir_stream_add_watch_full(input_stream->priv->stream,
> G_PRIORITY_DEFAULT,
> @@ -168,12 +163,8 @@ static void gvir_input_stream_read_async(GInputStream 
> *stream,
> g_object_ref(stream),
> (GDestroyNotify)g_object_unref);
>  
> -input_stream->priv->result =
> -g_simple_async_result_new(G_OBJE

[libvirt] [PATCH] lxc: drop sys_admin caps by default

[Cédric Bosdonnat]

To make sure the container user doesn't play with mounts, like
changing them from ro to rw, drop the sys_admin capability by default.
If user really needs to play with those, it can be enabled in the
configuration.
---
 Note: it seems that patch 3/3 or my last series never reached the list.
 Here it is.

 src/lxc/lxc_container.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index c5a70a1..d6d6fba 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -2083,6 +2083,7 @@ static int lxcContainerDropCapabilities(virDomainDefPtr 
def,
 case VIR_DOMAIN_CAPS_FEATURE_MKNOD: /* No creating device nodes */
 case VIR_DOMAIN_CAPS_FEATURE_AUDIT_CONTROL: /* No messing with 
auditing status */
 case VIR_DOMAIN_CAPS_FEATURE_MAC_ADMIN: /* No messing with LSM 
config */
+case VIR_DOMAIN_CAPS_FEATURE_SYS_ADMIN: /* No messing with mounts 
*/
 toDrop = (state != VIR_TRISTATE_SWITCH_ON);
 break;
 default: /* User specified capabilities to drop */
-- 
2.1.4

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

Re: [libvirt] [PATCH v3 0/7] qemu: Add support for -incoming defer

[Jiri Denemark]

On Wed, Nov 25, 2015 at 15:22:08 +0100, Pavel Hrdina wrote:
> On Wed, Nov 25, 2015 at 10:29:32AM +0100, Jiri Denemark wrote:
> > Traditionally, we pass incoming migration URI on QEMU command line,
> > which has some drawbacks. Depending on the URI QEMU may initialize its
> > migration state immediately without giving us a chance to set any
> > additional migration parameters (this applies mainly for fd: URIs). For
> > some URIs the monitor may be completely blocked from the beginning until
> > migration is finished, which means we may be stuck in qmp_capabilities
> > command without being able to send any QMP commands.
> > 
> > QEMU solved this by introducing "defer" parameter for -incoming command
> > line option. This will tell QEMU to prepare for an incoming migration
> > while the actual incoming URI is sent using migrate-incoming QMP
> > command. Before calling this command we can normally talk to the
> > monitor and even set any migration parameters which will be honored by
> > the incoming migration.
> > 
> > Jiri Denemark (7):
> >   qemu: Introduce qemuProcessInit
> >   qemu: Introduce qemuProcessLaunch
> >   qemu: Introduce qemuProcessFinishStartup
> >   qemu: Separate incoming URI generation from qemuMigrationPrepareAny
> >   qemu: Kill QEMU process if Prepare phase fails
> >   qemu: Skip starting NBD servers for offline migration
> >   qemu: Use qemuProcessLaunch in migration Prepare phase
> > 
> >  src/qemu/qemu_migration.c | 247 +--
> >  src/qemu/qemu_process.c   | 489 
> > +-
> >  src/qemu/qemu_process.h   |  20 ++
> >  3 files changed, 466 insertions(+), 290 deletions(-)
> 
> ACK series

Thanks, pushed.

Jirka

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

Re: [libvirt] [PATCH 0/8] Add perf and Intel CMT feature support

[Jiri Denemark]

On Wed, Nov 25, 2015 at 13:16:34 +, Ren, Qiaowei wrote:
> 
> > -Original Message-
> > From: Daniel P. Berrange [mailto:berra...@redhat.com]
> > Sent: Tuesday, November 24, 2015 9:29 PM
> > To: Ren, Qiaowei; libvir-list@redhat.com
> > Subject: Re: [libvirt] [PATCH 0/8] Add perf and Intel CMT feature support
> > 
> > On Tue, Nov 24, 2015 at 02:24:22PM +0100, Jiri Denemark wrote:
> > > On Tue, Nov 17, 2015 at 16:00:40 +0800, Qiaowei Ren wrote:
> > > > The series mainly adds Intel CMT feature support into libvirt. CMT
> > > > is new introduced PQos (Platform Qos) feature to monitor the usage
> > > > of cache by applications running on the platform.
> > > >
> > > > Currently CMT patches has been merged into Linux kernel mainline.
> > > > The CMT implementation in Linux kernel is based on perf mechanism
> > > > and there is no support for perf in libvirt, and so this series
> > > > firstly add perf support into libvirt, including two public API and
> > > > a set of util interfaces. And based on these APIs and interfaces,
> > > > thie series implements CMT perf event support.
> > > >
> > > > TODO:
> > > > 1. This series relys on keeping an open file descriptor for the guest.
> > > > We should add one patch to call sys_perf_event_open again iff
> > > > libvirtd is restarted.
> > >
> > > Yes, we should reenable perf events when reconnecting to running
> > > domains. Will we need to remember what events were enabled (in domain
> > > status XML) or is it something we can read back from the kernel?
> > 
> > We need to record it somewhere. I guess this raises the question of whether 
> > we
> > should hide it in status XML, or persist the list of desired perf events in 
> > the real
> > domain XML, so we can have the option of also having them enabled
> > immediately at startup, without requiring separate API call.
> > 
> 
> I checked the kernel perf interface and could not find the way to read
> back whether events were enabled from the kernel. So we have to record
> it somewhere according to Daniel.
> 
> If we just persist the list of desired perf events in the real domain
> XML, users can have the option of having them enabled at startup. But
> it is also possible that users enable more events  at runtime through
> API call for perf event. So should we record it in both real domain
> XML and status XML?

The status XML always contains full domain XML of the running domain.
Thus if you add to code to handle all this in domain XML, you don't need
to do anything extra to store the info in status XML. However, you'll
need to update live domain definition from the APIs which enable/disable
the perf events.

Jirka

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

Re: [libvirt] [PATCH v3 0/7] qemu: Add support for -incoming defer

[Pavel Hrdina]

On Wed, Nov 25, 2015 at 10:29:32AM +0100, Jiri Denemark wrote:
> Traditionally, we pass incoming migration URI on QEMU command line,
> which has some drawbacks. Depending on the URI QEMU may initialize its
> migration state immediately without giving us a chance to set any
> additional migration parameters (this applies mainly for fd: URIs). For
> some URIs the monitor may be completely blocked from the beginning until
> migration is finished, which means we may be stuck in qmp_capabilities
> command without being able to send any QMP commands.
> 
> QEMU solved this by introducing "defer" parameter for -incoming command
> line option. This will tell QEMU to prepare for an incoming migration
> while the actual incoming URI is sent using migrate-incoming QMP
> command. Before calling this command we can normally talk to the
> monitor and even set any migration parameters which will be honored by
> the incoming migration.
> 
> Jiri Denemark (7):
>   qemu: Introduce qemuProcessInit
>   qemu: Introduce qemuProcessLaunch
>   qemu: Introduce qemuProcessFinishStartup
>   qemu: Separate incoming URI generation from qemuMigrationPrepareAny
>   qemu: Kill QEMU process if Prepare phase fails
>   qemu: Skip starting NBD servers for offline migration
>   qemu: Use qemuProcessLaunch in migration Prepare phase
> 
>  src/qemu/qemu_migration.c | 247 +--
>  src/qemu/qemu_process.c   | 489 
> +-
>  src/qemu/qemu_process.h   |  20 ++
>  3 files changed, 466 insertions(+), 290 deletions(-)

ACK series

Pavel

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

Re: [libvirt] [PATCHv5 4/9] qemu: add support for hv_crash feature as a panic device

[Jiri Denemark]

On Tue, Nov 24, 2015 at 15:26:33 +0300, Dmitry Andreev wrote:
> Panic device type used depends on 'model' attribute.
> 
> If no model is specified then device type depends on hypervisor
> and guest arch. 'pseries' model is used for pSeries guest and
> 'isa' model is used in other cases.
> 
> XML:
> 
>   
> 
> 
> QEMU command line:
> qemu -cpu ,hv_crash
> ---
> v5:
> * autoselection of panic device model in post parse was moved
>   from another patch
> * ARCH_IS_X86 check for 'hyperv' panic device was added
> 
>  src/qemu/qemu_command.c | 60 
> -
>  src/qemu/qemu_domain.c  |  9 
>  2 files changed, 64 insertions(+), 5 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 4d00fd9..bb6d5fe 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
...
> @@ -11050,17 +11060,45 @@ qemuBuildCommandLine(virConnectPtr conn,
>  }
>  
>  if (def->panic) {
> -if (ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine, 
> "pseries")) {
> -/* For pSeries guests, the firmware provides the same
> - * functionality as the pvpanic device. The address
> +switch ((virDomainPanicModel) def->panic->model) {
> +case VIR_DOMAIN_PANIC_MODEL_HYPERV:
> +/* Panic with model 'hyperv' is not a device, it should
> + * be configured in cpu commandline. The address
>   * cannot be configured by the user */
> +if (!ARCH_IS_X86(def->os.arch)) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   _("only i686 and x86_64 architectures are 
> support "
> + "panic device of model 'hyperv'"));
> +goto error;
> +}
>  if (def->panic->info.type != 
> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
>  virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> _("setting the panic device address is not "
> - "supported for pSeries guests"));
> + "supported for model 'hyperv'"));
>  goto error;
>  }
> -} else {
> +break;
> +
> +case VIR_DOMAIN_PANIC_MODEL_PSERIES:
> +if (ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine, 
> "pseries")) {
> +/* For pSeries guests, the firmware provides the same
> + * functionality as the pvpanic device. The address
> + * cannot be configured by the user */
> +if (def->panic->info.type != 
> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   _("setting the panic device address is 
> not "
> + "supported for model 'pseries'"));
> +goto error;
> +}
> +} else {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +   _("only pSeries guests support panic device "
> + "of model 'pseries'"));
> +goto error;
> +}
> +break;

This could be reorganized a bit so that the flow is similar to heprv
panic model.

> +
> +case VIR_DOMAIN_PANIC_MODEL_ISA:
>  if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PANIC)) {
>  virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> _("the QEMU binary does not support the "

Some tests fail after this patch, the next patch (5/9) needs to be
squashed in this one to make them succeed again. The following patch
(6/9) is not necessary, but it is small enough and it logically fits
here, which makes it a good candidate for squashing.

ACK with patches 5/9 and 6/9, and the following diff squashed in.

Jirka

diff --git i/src/qemu/qemu_command.c w/src/qemu/qemu_command.c
index bb6d5fe..e83be58 100644
--- i/src/qemu/qemu_command.c
+++ w/src/qemu/qemu_command.c
@@ -11067,7 +11067,7 @@ qemuBuildCommandLine(virConnectPtr conn,
  * cannot be configured by the user */
 if (!ARCH_IS_X86(def->os.arch)) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("only i686 and x86_64 architectures are 
support "
+   _("only i686 and x86_64 guests support "
  "panic device of model 'hyperv'"));
 goto error;
 }
@@ -11080,22 +11080,22 @@ qemuBuildCommandLine(virConnectPtr conn,
 break;
 
 case VIR_DOMAIN_PANIC_MODEL_PSERIES:
-if (ARCH_IS_PPC64(def->os.arch) && STRPREFIX(def->os.machine, 
"pseries")) {
-/* For pSeries guests, the firmware provides the same
- * functionality as the pvpanic device

Re: [libvirt] [PATCHv5 9/9] conf: reject multiple panic devices of same model

[Jiri Denemark]

On Tue, Nov 24, 2015 at 15:26:38 +0300, Dmitry Andreev wrote:
> Only one panic device per model is allowed.
> ---
> v5: minor code fixes
> 
>  src/conf/domain_conf.c | 25 +
>  1 file changed, 25 insertions(+)

ACK

Jirka

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

Re: [libvirt] [PATCHv5 7/9] Allow multiple panic devices

[Jiri Denemark]

On Tue, Nov 24, 2015 at 15:26:36 +0300, Dmitry Andreev wrote:
> 'model' attribute was added to a panic device but only one panic
> device is allowed. This patch changes panic device presence
> from 'optional' to 'zeroOrMore'.
> ---
> v5: part of the code was moved out from this commit
> 
>  docs/formatdomain.html.in |  1 +
>  docs/schemas/domaincommon.rng |  4 ++--
>  src/conf/domain_conf.c| 53 
> ---
>  src/conf/domain_conf.h|  4 +++-
>  src/qemu/qemu_command.c   | 50 
>  src/qemu/qemu_domain.c| 21 +
>  6 files changed, 77 insertions(+), 56 deletions(-)

ACK with the following patch squashed in. And I think the next patch
which tests this new functionality is small enough that it would be nice
to have it squashed in this one.

Jirka

diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c
index d6977f2..2bddb67 100644
--- i/src/conf/domain_conf.c
+++ w/src/conf/domain_conf.c
@@ -18126,9 +18126,10 @@ virDomainDefCheckABIStability(virDomainDefPtr src,
 goto error;
 }
 
-for (i = 0; i < src->npanics; i++)
+for (i = 0; i < src->npanics; i++) {
 if (!virDomainPanicDefCheckABIStability(src->panics[i], 
dst->panics[i]))
 goto error;
+}
 
 if (src->nshmems != dst->nshmems) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,

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

Re: [libvirt] [PATCHv5 5/9] tests: rework tests for panic devices

[Jiri Denemark]

On Tue, Nov 24, 2015 at 15:26:34 +0300, Dmitry Andreev wrote:
> Panic device model will be selected automatically if it is not
> chosen by the user. Tests should cover this case.
> ---
>  .../qemuxml2argv-panic-no-address.xml  |  2 +-
>  .../qemuxml2argvdata/qemuxml2argv-pseries-disk.xml |  2 +-
>  .../qemuxml2argv-pseries-nvram.xml |  2 +-
>  tests/qemuxml2xmloutdata/qemuxml2xmlout-panic.xml  | 31 
> ++
>  .../qemuxml2xmlout-pseries-panic-missing.xml   |  2 +-
>  .../qemuxml2xmlout-pseries-panic-no-address.xml| 30 +
>  tests/qemuxml2xmltest.c|  4 +--
>  7 files changed, 67 insertions(+), 6 deletions(-)
>  create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-panic.xml
>  create mode 100644 
> tests/qemuxml2xmloutdata/qemuxml2xmlout-pseries-panic-no-address.xml

ACK if squashed in 4/9.

Jirka

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

Re: [libvirt] [PATCHv5 1/9] conf: refactor code for checking ABI stability of panic device

[Jiri Denemark]

On Tue, Nov 24, 2015 at 15:26:30 +0300, Dmitry Andreev wrote:
> ---
> v5: this code was moved from another patch
>  src/conf/domain_conf.c | 21 ++---
>  1 file changed, 2 insertions(+), 19 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 0ac7dbf..a14dd77 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -17619,7 +17619,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr 
> src,
>  }
>  
>  static bool
> -virDomainPanicCheckABIStability(virDomainPanicDefPtr src,
> +virDomainPanicDefCheckABIStability(virDomainPanicDefPtr src,
>  virDomainPanicDefPtr dst)
>  {
>  if (!src && !dst)
...

ACK with the following patch squashed in.

Jirka

diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c
index 4bbf941..7f4c643 100644
--- i/src/conf/domain_conf.c
+++ w/src/conf/domain_conf.c
@@ -17614,7 +17614,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr 
src,
 
 static bool
 virDomainPanicDefCheckABIStability(virDomainPanicDefPtr src,
-virDomainPanicDefPtr dst)
+   virDomainPanicDefPtr dst)
 {
 if (!src && !dst)
 return true;

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

Re: [libvirt] [PATCHv5 3/9] tests: add tests for the new panic device attribute - 'model'

[Jiri Denemark]

On Tue, Nov 24, 2015 at 15:26:32 +0300, Dmitry Andreev wrote:
> ---
> v5: tests was moved from another patch
> 
>  .../qemuxml2argvdata/qemuxml2argv-hyperv-panic.xml | 25 +
>  tests/qemuxml2argvdata/qemuxml2argv-panic-isa.xml  | 31 
> ++
>  .../qemuxml2argv-panic-pseries.xml | 30 +
>  tests/qemuxml2xmltest.c|  3 +++
>  4 files changed, 89 insertions(+)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.xml
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-isa.xml
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-pseries.xml

ACK

Jirka

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

Re: [libvirt] [PATCHv5 6/9] tests: add tests for the new 'hyperv' panic device model

[Jiri Denemark]

On Tue, Nov 24, 2015 at 15:26:35 +0300, Dmitry Andreev wrote:
> ---
>  tests/qemuargv2xmltest.c|  1 +
>  .../qemuxml2argvdata/qemuxml2argv-hyperv-panic.args | 21 
> +
>  tests/qemuxml2argvtest.c|  1 +
>  3 files changed, 23 insertions(+)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hyperv-panic.args

ACK if squashed in 4/9.

Jirka

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

Re: [libvirt] [PATCHv5 0/9] Hyper-v crash feature support

[Jiri Denemark]

On Tue, Nov 24, 2015 at 15:26:29 +0300, Dmitry Andreev wrote:
> A new Hyper-V cpu feature 'hv_crash' was added to QEMU. The feature
> will become available in v2.5.0.
> 
> What is changed in v5:
> * minor code fixes
> * code was moved between patches
> * patch sequence changed
> 
> Dmitry Andreev (9):
>   conf: refactor code for checking ABI stability of panic device
>   conf: add 'model' attribute for panic device with values isa, pseries,
> hyperv
>   tests: add tests for the new panic device attribute - 'model'
>   qemu: add support for hv_crash feature as a panic device
>   tests: rework tests for panic devices
>   tests: add tests for the new 'hyperv' panic device model
>   Allow multiple panic devices
>   tests: add tests for multiple panic devices
>   conf: reject multiple panic devices of same model

Thanks for the good work and patience. I think the series is pretty good
now. I found a few nits which are not really worth a respin so I fixed
them (see my replies to individual patches) and pushed the series.

Jirka

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

Re: [libvirt] [PATCHv5 2/9] conf: add 'model' attribute for panic device with values isa, pseries, hyperv

[Jiri Denemark]

On Tue, Nov 24, 2015 at 15:26:31 +0300, Dmitry Andreev wrote:
> Libvirt already has two types of panic devices - pvpanic and pSeries firmware.
> This patch introduces the 'model' attribute and a new type of panic device.
> 
> 'isa' model is for ISA pvpanic device.
> 'pseries' model is a default value for pSeries guests.
> 'hyperv' model is the new type. It's used for Hyper-V crash.
> 
> Schema and docs are updated for the new attribute.
> ---
> v5: minor fixes
>  
>  docs/formatdomain.html.in | 18 --
>  docs/schemas/domaincommon.rng |  9 +
>  src/conf/domain_conf.c| 34 +-
>  src/conf/domain_conf.h| 11 +++
>  4 files changed, 69 insertions(+), 3 deletions(-)

ACK

Jirka

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

Re: [libvirt] [PATCHv5 8/9] tests: add tests for multiple panic devices

[Jiri Denemark]

On Tue, Nov 24, 2015 at 15:26:37 +0300, Dmitry Andreev wrote:
> ---
>  .../qemuxml2argv-panic-double.args | 21 
>  .../qemuxml2argvdata/qemuxml2argv-panic-double.xml | 28 
> ++
>  tests/qemuxml2argvtest.c   |  2 ++
>  tests/qemuxml2xmltest.c|  1 +
>  4 files changed, 52 insertions(+)
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-double.args
>  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-panic-double.xml

ACK if squashed in 7/9.

Jirka

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

[libvirt] [PATCH v2] Allow building lxc without virt-login-shell

[Cédric Bosdonnat]

Add a configure option to disable virt-login-shell build even if lxc is
enabled.
---
Diff to V1:

 * moved the configure.ac code into m4/virt-login-shell.m4
 * allow building virt-login-shell without lxc
 * Introduce WITH_SETUID_RPC_CLIENT to have it built if
   either lxc or virt-login-shell is built.

 configure.ac   |  9 +
 m4/virt-login-shell.m4 | 27 +++
 src/Makefile.am|  4 ++--
 tools/Makefile.am  | 12 ++--
 4 files changed, 44 insertions(+), 8 deletions(-)
 create mode 100644 m4/virt-login-shell.m4

diff --git a/configure.ac b/configure.ac
index f481c50..6fef728 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1075,6 +1075,14 @@ fi
 AM_CONDITIONAL([WITH_LXC], [test "$with_lxc" = "yes"])
 
 dnl
+dnl Check for virt-login-shell
+dnl
+
+LIBVIRT_CHECK_LOGIN_SHELL
+
+AM_CONDITIONAL([WITH_SETUID_RPC_CLIENT], [test "$with_lxc$with_login_shell" != 
"nono"])
+
+dnl
 dnl Checks for the Parallels driver
 dnl
 
@@ -2974,6 +2982,7 @@ AC_MSG_NOTICE([  Init script: $with_init_script])
 AC_MSG_NOTICE([Char device locks: $with_chrdev_lock_files])
 AC_MSG_NOTICE([   Default Editor: $DEFAULT_EDITOR])
 AC_MSG_NOTICE([ Loader/NVRAM: $with_loader_nvram])
+AC_MSG_NOTICE([ virt-login-shell: $with_login_shell])
 AC_MSG_NOTICE([])
 AC_MSG_NOTICE([Developer Tools])
 AC_MSG_NOTICE([])
diff --git a/m4/virt-login-shell.m4 b/m4/virt-login-shell.m4
new file mode 100644
index 000..0701054
--- /dev/null
+++ b/m4/virt-login-shell.m4
@@ -0,0 +1,27 @@
+dnl Copyright (c) 2015 SUSE LINUX Products GmbH, Nuernberg, Germany.
+dnl
+dnl This library is free software; you can redistribute it and/or
+dnl modify it under the terms of the GNU Lesser General Public
+dnl License as published by the Free Software Foundation; either
+dnl version 2.1 of the License, or (at your option) any later version.
+dnl
+dnl This library is distributed in the hope that it will be useful,
+dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
+dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+dnl Lesser General Public License for more details.
+dnl
+dnl You should have received a copy of the GNU Lesser General Public
+dnl License along with this library.  If not, see
+dnl .
+
+AC_DEFUN([LIBVIRT_CHECK_LOGIN_SHELL], [
+  AC_ARG_WITH([login_shell],
+[AS_HELP_STRING([--with-login-shell],
+  [build virt-login-shell @<:@default=yes@:>@])])
+  m4_divert_text([DEFAULTS], [with_login_shell=yes])
+
+  if test "$with_login_shell" ; then
+  AC_DEFINE_UNQUOTED([WITH_LOGIN_SHELL], 1, [whether virt-login-shell is 
built])
+  fi
+  AM_CONDITIONAL([WITH_LOGIN_SHELL], [test "$with_login_shell" = "yes"])
+])
diff --git a/src/Makefile.am b/src/Makefile.am
index 99b4993..2f40910 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2225,7 +2225,7 @@ libvirt_lxc_la_LIBADD = libvirt.la $(CYGWIN_EXTRA_LIBADD)
 # have a RPC client for local UNIX socket access only. We use
 # the ../config-post.h header to disable all external deps that
 # we don't want
-if WITH_LXC
+if WITH_SETUID_RPC_CLIENT
 noinst_LTLIBRARIES += libvirt-setuid-rpc-client.la
 
 libvirt_setuid_rpc_client_la_SOURCES = \
@@ -2302,7 +2302,7 @@ libvirt_setuid_rpc_client_la_CFLAGS = \
$(SECDRIVER_CFLAGS) \
$(XDR_CFLAGS)   \
$(NULL)
-endif WITH_LXC
+endif WITH_SETUID_RPC_CLIENT
 
 lockdriverdir = $(libdir)/libvirt/lock-driver
 lockdriver_LTLIBRARIES =
diff --git a/tools/Makefile.am b/tools/Makefile.am
index d5638d9..d005035 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -71,12 +71,12 @@ sbin_SCRIPTS = virt-sanlock-cleanup
 DISTCLEANFILES += virt-sanlock-cleanup
 endif WITH_SANLOCK
 
-if WITH_LXC
+if WITH_LOGIN_SHELL
 conf_DATA += virt-login-shell.conf
 bin_PROGRAMS += virt-login-shell
-else ! WITH_LXC
+else ! WITH_LOGIN_SHELL
 EXTRA_DIST += virt-login-shell.conf
-endif ! WITH_LXC
+endif ! WITH_LOGIN_SHELL
 
 
 dist_man1_MANS = \
@@ -84,11 +84,11 @@ dist_man1_MANS = \
virt-pki-validate.1 \
virt-xml-validate.1 \
virsh.1
-if WITH_LXC
+if WITH_LOGIN_SHELL
 dist_man1_MANS += virt-login-shell.1
-else ! WITH_LXC
+else ! WITH_LOGIN_SHELL
 EXTRA_DIST += virt-login-shell.1
-endif ! WITH_LXC
+endif ! WITH_LOGIN_SHELL
 if WITH_SANLOCK
 dist_man8_MANS = virt-sanlock-cleanup.8
 endif WITH_SANLOCK
-- 
2.1.4

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

Re: [libvirt] [PATCH] hypervisor driver for Jailhouse

[Christian Loehle]

The suggestions in your review should all be implemented now.

--
Christian Loehle

diff --git a/configure.ac b/configure.ac
index 4b7c9ed..02db58e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1087,6 +1087,12 @@ dnl
 LIBVIRT_DRIVER_CHECK_BHYVE
 
 dnl
+dnl Checks for Jailhouse driver
+dnl
+
+LIBVIRT_DRIVER_CHECK_JAILHOUSE
+
+dnl
 dnl check for shell that understands <> redirection without truncation,
 dnl needed by src/qemu/qemu_monitor_{text,json}.c.
 dnl
@@ -2830,6 +2836,7 @@ AC_MSG_NOTICE([  ESX: $with_esx])
 AC_MSG_NOTICE([  Hyper-V: $with_hyperv])
 LIBVIRT_DRIVER_RESULT_VZ
 LIBVIRT_DRIVER_RESULT_BHYVE
+LIBVIRT_DRIVER_RESULT_JAILHOUSE
 AC_MSG_NOTICE([ Test: $with_test])
 AC_MSG_NOTICE([   Remote: $with_remote])
 AC_MSG_NOTICE([  Network: $with_network])
diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index f716cb9..c8fe2d3 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -127,6 +127,7 @@ typedef enum {
 VIR_FROM_POLKIT = 60,   /* Error from polkit code */
 VIR_FROM_THREAD = 61,   /* Error from thread utils */
 VIR_FROM_ADMIN = 62,/* Error from admin backend */
+VIR_FROM_JAILHOUSE = 63,   /* Error from Jailhouse driver */
 
 # ifdef VIR_ENUM_SENTINELS
 VIR_ERR_DOMAIN_LAST
diff --git a/libvirt-jailhousev2.patch b/libvirt-jailhousev2.patch
new file mode 100644
index 000..e69de29
diff --git a/m4/virt-driver-jailhouse.m4 b/m4/virt-driver-jailhouse.m4
new file mode 100644
index 000..65d53f2
--- /dev/null
+++ b/m4/virt-driver-jailhouse.m4
@@ -0,0 +1,31 @@
+dnl The Jailhouse driver
+dnl
+dnl Copyright (C) 2005-2015 Red Hat, Inc.
+dnl
+dnl This library is free software; you can redistribute it and/or
+dnl modify it under the terms of the GNU Lesser General Public
+dnl License as published by the Free Software Foundation; either
+dnl version 2.1 of the License, or (at your option) any later version.
+dnl
+dnl This library is distributed in the hope that it will be useful,
+dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
+dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+dnl Lesser General Public License for more details.
+dnl
+dnl You should have received a copy of the GNU Lesser General Public
+dnl License along with this library.  If not, see
+dnl .
+dnl
+
+AC_DEFUN([LIBVIRT_DRIVER_CHECK_JAILHOUSE],[
+AC_ARG_WITH([jailhouse],
+  [AS_HELP_STRING([--with-jailhouse],
+[add Jailhouse support @<:@default=yes@:>@])])
+m4_divert_text([DEFAULTS], [with_jailhouse=yes])
+
+AM_CONDITIONAL([WITH_JAILHOUSE], [test "$with_jailhouse" = "yes"])
+])
+
+AC_DEFUN([LIBVIRT_DRIVER_RESULT_JAILHOUSE],[
+AC_MSG_NOTICE([Jailhouse: $with_jailhouse])
+])
diff --git a/po/POTFILES.in b/po/POTFILES.in
index 0cc5b99..2b144bf 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -59,6 +59,7 @@ src/hyperv/hyperv_wmi.c
 src/interface/interface_backend_netcf.c
 src/interface/interface_backend_udev.c
 src/internal.h
+src/jailhouse/jailhouse_driver.c
 src/libvirt.c
 src/libvirt-admin.c
 src/libvirt-domain.c
diff --git a/src/Makefile.am b/src/Makefile.am
index 99b4993..10d59de 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -578,6 +578,7 @@ DRIVER_SOURCE_FILES = \
$(VMWARE_DRIVER_SOURCES) \
$(XEN_DRIVER_SOURCES) \
$(XENAPI_DRIVER_SOURCES) \
+   $(JAILHOUSE_DRIVER_SOURCES) \
$(NULL)
 
 STATEFUL_DRIVER_SOURCE_FILES = \
@@ -860,6 +861,11 @@ BHYVE_DRIVER_SOURCES = 
\
bhyve/bhyve_utils.h \
$(NULL)
 
+JAILHOUSE_DRIVER_SOURCES = \
+   jailhouse/jailhouse_driver.c\
+   jailhouse/jailhouse_driver.h\
+   $(NULL)
+
 NETWORK_DRIVER_SOURCES =   \
network/bridge_driver.h network/bridge_driver.c \
network/bridge_driver_platform.h\
@@ -1436,6 +1442,14 @@ libvirt_driver_vz_la_LIBADD = $(PARALLELS_SDK_LIBS) 
$(LIBNL_LIBS)
 libvirt_driver_vz_la_SOURCES = $(VZ_DRIVER_SOURCES)
 endif WITH_VZ
 
+if WITH_JAILHOUSE
+noinst_LTLIBRARIES += libvirt_driver_jailhouse.la
+libvirt_la_BUILT_LIBADD += libvirt_driver_jailhouse.la
+libvirt_driver_jailhouse_la_CFLAGS = \
+   -I$(srcdir)/conf $(AM_CFLAGS)
+libvirt_driver_jailhouse_la_SOURCES = $(JAILHOUSE_DRIVER_SOURCES)
+endif WITH_JAILHOUSE
+
 if WITH_BHYVE
 noinst_LTLIBRARIES += libvirt_driver_bhyve_impl.la
 libvirt_driver_bhyve_la_SOURCES =
@@ -1801,6 +1815,7 @@ EXTRA_DIST += 
\
$(HYPERV_DRIVER_EXTRA_DIST) \
$(VZ_DRIVER_SOURCES)\
$(BHYVE_DRIVER_SOURCES) \
+   $(JAILHOUSE_DRIVER_SOURCES) 

Re: [libvirt] [PATCH 0/8] Add perf and Intel CMT feature support

[Ren, Qiaowei]

> -Original Message-
> From: Daniel P. Berrange [mailto:berra...@redhat.com]
> Sent: Tuesday, November 24, 2015 9:29 PM
> To: Ren, Qiaowei; libvir-list@redhat.com
> Subject: Re: [libvirt] [PATCH 0/8] Add perf and Intel CMT feature support
> 
> On Tue, Nov 24, 2015 at 02:24:22PM +0100, Jiri Denemark wrote:
> > On Tue, Nov 17, 2015 at 16:00:40 +0800, Qiaowei Ren wrote:
> > > The series mainly adds Intel CMT feature support into libvirt. CMT
> > > is new introduced PQos (Platform Qos) feature to monitor the usage
> > > of cache by applications running on the platform.
> > >
> > > Currently CMT patches has been merged into Linux kernel mainline.
> > > The CMT implementation in Linux kernel is based on perf mechanism
> > > and there is no support for perf in libvirt, and so this series
> > > firstly add perf support into libvirt, including two public API and
> > > a set of util interfaces. And based on these APIs and interfaces,
> > > thie series implements CMT perf event support.
> > >
> > > TODO:
> > > 1. This series relys on keeping an open file descriptor for the guest.
> > > We should add one patch to call sys_perf_event_open again iff
> > > libvirtd is restarted.
> >
> > Yes, we should reenable perf events when reconnecting to running
> > domains. Will we need to remember what events were enabled (in domain
> > status XML) or is it something we can read back from the kernel?
> 
> We need to record it somewhere. I guess this raises the question of whether we
> should hide it in status XML, or persist the list of desired perf events in 
> the real
> domain XML, so we can have the option of also having them enabled
> immediately at startup, without requiring separate API call.
> 

I checked the kernel perf interface and could not find the way to read back 
whether events were enabled from the kernel. So we have to record it somewhere 
according to Daniel.

If we just persist the list of desired perf events in the real domain XML, 
users can have the option of having them enabled at startup. But it is also 
possible that users enable more events  at runtime through API call for perf 
event. So should we record it in both real domain XML and status XML?

Thanks,
Qiaowei

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

Re: [libvirt] "migration_address must not be the address of the local machine: 127.0.0.1"

[Laszlo Ersek]

On 11/25/15 12:10, Daniel P. Berrange wrote:
> On Wed, Nov 25, 2015 at 12:07:00PM +0100, Laszlo Ersek wrote:
>> On 11/25/15 12:00, Daniel P. Berrange wrote:
>>> On Wed, Nov 25, 2015 at 11:52:21AM +0100, Laszlo Ersek wrote:
 I recently upgraded my laptop from RHEL-7.1 to RHEL-7.2.

 I always pay attention to *.rpmnew config files, and I manually diff and
 merge them with the ones I have in place.

 I did the same with "/etc/libvirt/qemu.conf" this time.

 Now libvirtd doesn't start for me. Systemd doesn't actually notice the
 startup failure (insert bitter joke about systemd being so much better
 than startup scripts); it only reports the service inactive/dead (=
 unstarted), rather than failed.

 But, the libvirtd log file gives the reason:

 migration_address must not be the address of the local machine:
 127.0.0.1

 The error is easy to fix up in the config file, but my question is:

 Why must migration_address not be the address of the local machine?
>>>
>>> The migration address for incoming migration over TCP needs to be
>>> a public facing IP address, otherwise the remote machine won't be
>>> able to connect to it. If you configure migration_address on the
>>> target machine to be 127.0.0.1, then obviously no migration client
>>> connection will ever succeed, hence we consider 127.0.0.1 as an
>>> invalid configuration.
>>>
 BTW, my purpose is not in-host migration (perhaps that's indeed
 unsupported, I don't know); I just want to lock down the incoming
 migration port (and not just with firewall rules).
>>>
>>> What's wrong with using firewall rules ? IMHO you are describing
>>> exactly the scenario that are intended to deal with.
>>
>> I certainly use firewall rules.
>>
>> But, I like to disable listeners, especially public listeners, on the
>> individual application level too, if I don't have a good use for the
>> service.
> 
> NB, nothing will ever listen on the migration_address unless you
> actually trigger a migration to the host in question.

Ah, great. Jirka said the same. Thank you both.

Cheers
Laszlo

> So if you
> have authentication required to connect to libvirt you'll be
> fine unless the person using libvirt asks to migrate a VM to
> that host. An authenticated connection to libvirt should be
> considered equivalent to having root access regardless, so from
> that POV having migrate_address point to a public IP is not
> opening you up to any attack vector that doesn't also exist
> when you have it set to 127.0.0.1. So I still think restricting
> the address to 127.0.0.1 is not adding you any actual security
> benefit.
> 
> Regards,
> Daniel
> 

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

Re: [libvirt] "migration_address must not be the address of the local machine: 127.0.0.1"

[Jiri Denemark]

On Wed, Nov 25, 2015 at 12:07:00 +0100, Laszlo Ersek wrote:
> On 11/25/15 12:00, Daniel P. Berrange wrote:
> > On Wed, Nov 25, 2015 at 11:52:21AM +0100, Laszlo Ersek wrote:
> >> I recently upgraded my laptop from RHEL-7.1 to RHEL-7.2.
> >>
> >> I always pay attention to *.rpmnew config files, and I manually diff and
> >> merge them with the ones I have in place.
> >>
> >> I did the same with "/etc/libvirt/qemu.conf" this time.
> >>
> >> Now libvirtd doesn't start for me. Systemd doesn't actually notice the
> >> startup failure (insert bitter joke about systemd being so much better
> >> than startup scripts); it only reports the service inactive/dead (=
> >> unstarted), rather than failed.
> >>
> >> But, the libvirtd log file gives the reason:
> >>
> >> migration_address must not be the address of the local machine:
> >> 127.0.0.1
> >>
> >> The error is easy to fix up in the config file, but my question is:
> >>
> >> Why must migration_address not be the address of the local machine?
> > 
> > The migration address for incoming migration over TCP needs to be
> > a public facing IP address, otherwise the remote machine won't be
> > able to connect to it. If you configure migration_address on the
> > target machine to be 127.0.0.1, then obviously no migration client
> > connection will ever succeed, hence we consider 127.0.0.1 as an
> > invalid configuration.
> > 
> >> BTW, my purpose is not in-host migration (perhaps that's indeed
> >> unsupported, I don't know); I just want to lock down the incoming
> >> migration port (and not just with firewall rules).
> > 
> > What's wrong with using firewall rules ? IMHO you are describing
> > exactly the scenario that are intended to deal with.
> 
> I certainly use firewall rules.
> 
> But, I like to disable listeners, especially public listeners, on the
> individual application level too, if I don't have a good use for the
> service.

There will be no listener unless you start migrating to your host.

Jirka

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

Re: [libvirt] "migration_address must not be the address of the local machine: 127.0.0.1"

[Daniel P. Berrange]

On Wed, Nov 25, 2015 at 12:07:00PM +0100, Laszlo Ersek wrote:
> On 11/25/15 12:00, Daniel P. Berrange wrote:
> > On Wed, Nov 25, 2015 at 11:52:21AM +0100, Laszlo Ersek wrote:
> >> I recently upgraded my laptop from RHEL-7.1 to RHEL-7.2.
> >>
> >> I always pay attention to *.rpmnew config files, and I manually diff and
> >> merge them with the ones I have in place.
> >>
> >> I did the same with "/etc/libvirt/qemu.conf" this time.
> >>
> >> Now libvirtd doesn't start for me. Systemd doesn't actually notice the
> >> startup failure (insert bitter joke about systemd being so much better
> >> than startup scripts); it only reports the service inactive/dead (=
> >> unstarted), rather than failed.
> >>
> >> But, the libvirtd log file gives the reason:
> >>
> >> migration_address must not be the address of the local machine:
> >> 127.0.0.1
> >>
> >> The error is easy to fix up in the config file, but my question is:
> >>
> >> Why must migration_address not be the address of the local machine?
> > 
> > The migration address for incoming migration over TCP needs to be
> > a public facing IP address, otherwise the remote machine won't be
> > able to connect to it. If you configure migration_address on the
> > target machine to be 127.0.0.1, then obviously no migration client
> > connection will ever succeed, hence we consider 127.0.0.1 as an
> > invalid configuration.
> > 
> >> BTW, my purpose is not in-host migration (perhaps that's indeed
> >> unsupported, I don't know); I just want to lock down the incoming
> >> migration port (and not just with firewall rules).
> > 
> > What's wrong with using firewall rules ? IMHO you are describing
> > exactly the scenario that are intended to deal with.
> 
> I certainly use firewall rules.
> 
> But, I like to disable listeners, especially public listeners, on the
> individual application level too, if I don't have a good use for the
> service.

NB, nothing will ever listen on the migration_address unless you
actually trigger a migration to the host in question. So if you
have authentication required to connect to libvirt you'll be
fine unless the person using libvirt asks to migrate a VM to
that host. An authenticated connection to libvirt should be
considered equivalent to having root access regardless, so from
that POV having migrate_address point to a public IP is not
opening you up to any attack vector that doesn't also exist
when you have it set to 127.0.0.1. So I still think restricting
the address to 127.0.0.1 is not adding you any actual security
benefit.

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] "migration_address must not be the address of the local machine: 127.0.0.1"

[Laszlo Ersek]

On 11/25/15 12:00, Daniel P. Berrange wrote:
> On Wed, Nov 25, 2015 at 11:52:21AM +0100, Laszlo Ersek wrote:
>> I recently upgraded my laptop from RHEL-7.1 to RHEL-7.2.
>>
>> I always pay attention to *.rpmnew config files, and I manually diff and
>> merge them with the ones I have in place.
>>
>> I did the same with "/etc/libvirt/qemu.conf" this time.
>>
>> Now libvirtd doesn't start for me. Systemd doesn't actually notice the
>> startup failure (insert bitter joke about systemd being so much better
>> than startup scripts); it only reports the service inactive/dead (=
>> unstarted), rather than failed.
>>
>> But, the libvirtd log file gives the reason:
>>
>> migration_address must not be the address of the local machine:
>> 127.0.0.1
>>
>> The error is easy to fix up in the config file, but my question is:
>>
>> Why must migration_address not be the address of the local machine?
> 
> The migration address for incoming migration over TCP needs to be
> a public facing IP address, otherwise the remote machine won't be
> able to connect to it. If you configure migration_address on the
> target machine to be 127.0.0.1, then obviously no migration client
> connection will ever succeed, hence we consider 127.0.0.1 as an
> invalid configuration.
> 
>> BTW, my purpose is not in-host migration (perhaps that's indeed
>> unsupported, I don't know); I just want to lock down the incoming
>> migration port (and not just with firewall rules).
> 
> What's wrong with using firewall rules ? IMHO you are describing
> exactly the scenario that are intended to deal with.

I certainly use firewall rules.

But, I like to disable listeners, especially public listeners, on the
individual application level too, if I don't have a good use for the
service.

>> If there's a way to disable incoming migration in libvirtd, I'd be
>> interested in that.
> 
> You could setup libvirt's API access control rules to deny the
> "migrate" privilege to all users.  Using firewall rules is a
> more secure solution though IMHO

I agree about that.

Thanks
Laszlo

> 
> Regards,
> Daniel
> 

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

Re: [libvirt] "migration_address must not be the address of the local machine: 127.0.0.1"

[Daniel P. Berrange]

On Wed, Nov 25, 2015 at 11:52:21AM +0100, Laszlo Ersek wrote:
> I recently upgraded my laptop from RHEL-7.1 to RHEL-7.2.
> 
> I always pay attention to *.rpmnew config files, and I manually diff and
> merge them with the ones I have in place.
> 
> I did the same with "/etc/libvirt/qemu.conf" this time.
> 
> Now libvirtd doesn't start for me. Systemd doesn't actually notice the
> startup failure (insert bitter joke about systemd being so much better
> than startup scripts); it only reports the service inactive/dead (=
> unstarted), rather than failed.
> 
> But, the libvirtd log file gives the reason:
> 
> migration_address must not be the address of the local machine:
> 127.0.0.1
> 
> The error is easy to fix up in the config file, but my question is:
> 
> Why must migration_address not be the address of the local machine?

The migration address for incoming migration over TCP needs to be
a public facing IP address, otherwise the remote machine won't be
able to connect to it. If you configure migration_address on the
target machine to be 127.0.0.1, then obviously no migration client
connection will ever succeed, hence we consider 127.0.0.1 as an
invalid configuration.

> BTW, my purpose is not in-host migration (perhaps that's indeed
> unsupported, I don't know); I just want to lock down the incoming
> migration port (and not just with firewall rules).

What's wrong with using firewall rules ? IMHO you are describing
exactly the scenario that are intended to deal with.

> If there's a way to disable incoming migration in libvirtd, I'd be
> interested in that.

You could setup libvirt's API access control rules to deny the
"migrate" privilege to all users.  Using firewall rules is a
more secure solution though IMHO

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] "migration_address must not be the address of the local machine: 127.0.0.1"

[Laszlo Ersek]

I recently upgraded my laptop from RHEL-7.1 to RHEL-7.2.

I always pay attention to *.rpmnew config files, and I manually diff and
merge them with the ones I have in place.

I did the same with "/etc/libvirt/qemu.conf" this time.

Now libvirtd doesn't start for me. Systemd doesn't actually notice the
startup failure (insert bitter joke about systemd being so much better
than startup scripts); it only reports the service inactive/dead (=
unstarted), rather than failed.

But, the libvirtd log file gives the reason:

migration_address must not be the address of the local machine:
127.0.0.1

The error is easy to fix up in the config file, but my question is:

Why must migration_address not be the address of the local machine?

This comes from:

commit 5e0561e115de10da342296bb7c7361e91e368d9c
Author: Chen Fan 
Date:   Tue Oct 7 12:07:32 2014 +0800

conf: Check whether migration_address is localhost

When enabling the migration_address option, by default it is
set to "127.0.0.1", but it's not a valid address for migration.
so we should add verification and set the default migration_address
to "0.0.0.0".

Signed-off-by: Chen Fan 
Signed-off-by: Ján Tomko 

I checked the mailing list archive. AFAICS, the earliest appearance of
this check is:

https://www.redhat.com/archives/libvir-list/2014-August/msg01503.html

The validity of the proposed check was never questioned, nor explained.

What gives?

BTW, my purpose is not in-host migration (perhaps that's indeed
unsupported, I don't know); I just want to lock down the incoming
migration port (and not just with firewall rules).

If there's a way to disable incoming migration in libvirtd, I'd be
interested in that.

Thanks!
Laszlo

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

[libvirt] [PATCH v3 6/7] qemu: Skip starting NBD servers for offline migration

[Jiri Denemark]

NBD storage migration will not work with offline migration anyway and we
already checked that the user did not ask for it. Thus it doesn't make
sense to keep the code after 'done' label where we jump in case of
offline migration.

Signed-off-by: Jiri Denemark 
---

Notes:
ACKed in version 1

Version 3:
- no change

Version 2:
- no change

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

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 9a4f19f..b6525df 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3418,6 +3418,9 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
  "make sense"));
 goto cleanup;
 }
+cookieFlags = 0;
+} else {
+cookieFlags = QEMU_MIGRATION_COOKIE_GRAPHICS;
 }
 
 if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
@@ -3572,6 +3575,16 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
QEMU_ASYNC_JOB_MIGRATION_IN) < 0)
 goto stopjob;
 
+if (mig->nbd &&
+flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) &&
+virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NBD_SERVER)) {
+if (qemuMigrationStartNBDServer(driver, vm, listenAddress,
+nmigrate_disks, migrate_disks) < 0) {
+goto stopjob;
+}
+cookieFlags |= QEMU_MIGRATION_COOKIE_NBD;
+}
+
 if (mig->lockState) {
 VIR_DEBUG("Received lockstate %s", mig->lockState);
 VIR_FREE(priv->lockState);
@@ -3582,22 +3595,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 }
 
  done:
-if (flags & VIR_MIGRATE_OFFLINE)
-cookieFlags = 0;
-else
-cookieFlags = QEMU_MIGRATION_COOKIE_GRAPHICS;
-
-if (mig->nbd &&
-flags & (VIR_MIGRATE_NON_SHARED_DISK | VIR_MIGRATE_NON_SHARED_INC) &&
-virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NBD_SERVER)) {
-if (qemuMigrationStartNBDServer(driver, vm, listenAddress,
-nmigrate_disks, migrate_disks) < 0) {
-/* error already reported */
-goto stopjob;
-}
-cookieFlags |= QEMU_MIGRATION_COOKIE_NBD;
-}
-
 if (qemuMigrationBakeCookie(mig, driver, vm, cookieout,
 cookieoutlen, cookieFlags) < 0) {
 /* We could tear down the whole guest here, but
-- 
2.6.3

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

[libvirt] [PATCH v3 3/7] qemu: Introduce qemuProcessFinishStartup

[Jiri Denemark]

Finishes starting a new domain launched by qemuProcessLaunch.

Signed-off-by: Jiri Denemark 
---

Notes:
Version 3:
- propagate asyncJob into qemuProcessFinishStartup to avoid effectively
  reverting 668a0fe fix from Jan

Version 2:
- save status before running VIR_HOOK_QEMU_OP_STARTED hook
- rename qemuProcessFinish as qemuProcessFinishStartup
- remove unused cfg from qemuProcessStart

 src/qemu/qemu_process.c | 79 -
 src/qemu/qemu_process.h |  7 +
 2 files changed, 59 insertions(+), 27 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 1b3cbc0..1fe551b 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5065,6 +5065,53 @@ qemuProcessLaunch(virConnectPtr conn,
 }
 
 
+/**
+ * qemuProcessFinishStartup:
+ *
+ * Finish starting a new domain.
+ */
+int
+qemuProcessFinishStartup(virConnectPtr conn,
+ virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ qemuDomainAsyncJob asyncJob,
+ bool startCPUs,
+ virDomainPausedReason pausedReason)
+{
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+int ret = -1;
+
+if (startCPUs) {
+VIR_DEBUG("Starting domain CPUs");
+if (qemuProcessStartCPUs(driver, vm, conn,
+ VIR_DOMAIN_RUNNING_BOOTED,
+ asyncJob) < 0) {
+if (!virGetLastError())
+virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+   _("resume operation failed"));
+goto cleanup;
+}
+} else {
+virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, pausedReason);
+}
+
+VIR_DEBUG("Writing domain status to disk");
+if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
+goto cleanup;
+
+if (qemuProcessStartHook(driver, vm,
+ VIR_HOOK_QEMU_OP_STARTED,
+ VIR_HOOK_SUBOP_BEGIN) < 0)
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+virObjectUnref(cfg);
+return ret;
+}
+
+
 int
 qemuProcessStart(virConnectPtr conn,
  virQEMUDriverPtr driver,
@@ -5077,7 +5124,6 @@ qemuProcessStart(virConnectPtr conn,
  virNetDevVPortProfileOp vmop,
  unsigned int flags)
 {
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 qemuDomainObjPrivatePtr priv = vm->privateData;
 qemuProcessIncomingDefPtr incoming = NULL;
 unsigned int stopFlags;
@@ -5120,31 +5166,11 @@ qemuProcessStart(virConnectPtr conn,
 qemuMigrationRunIncoming(driver, vm, incoming->deferredURI, asyncJob) 
< 0)
 goto stop;
 
-if (!(flags & VIR_QEMU_PROCESS_START_PAUSED)) {
-VIR_DEBUG("Starting domain CPUs");
-/* Allow the CPUS to start executing */
-if (qemuProcessStartCPUs(driver, vm, conn,
- VIR_DOMAIN_RUNNING_BOOTED,
- asyncJob) < 0) {
-if (virGetLastError() == NULL)
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("resume operation failed"));
-goto stop;
-}
-} else {
-virDomainObjSetState(vm, VIR_DOMAIN_PAUSED,
- incoming ?
- VIR_DOMAIN_PAUSED_MIGRATION :
- VIR_DOMAIN_PAUSED_USER);
-}
-
-VIR_DEBUG("Writing domain status to disk");
-if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
-goto stop;
-
-if (qemuProcessStartHook(driver, vm,
- VIR_HOOK_QEMU_OP_STARTED,
- VIR_HOOK_SUBOP_BEGIN) < 0)
+if (qemuProcessFinishStartup(conn, driver, vm, asyncJob,
+ !(flags & VIR_QEMU_PROCESS_START_PAUSED),
+ incoming ?
+ VIR_DOMAIN_PAUSED_MIGRATION :
+ VIR_DOMAIN_PAUSED_USER) < 0)
 goto stop;
 
 /* Keep watching qemu log for errors during incoming migration, otherwise
@@ -5155,7 +5181,6 @@ qemuProcessStart(virConnectPtr conn,
 ret = 0;
 
  cleanup:
-virObjectUnref(cfg);
 qemuProcessIncomingDefFree(incoming);
 return ret;
 
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index 54009c5..978aa3a 100644
--- a/src/qemu/qemu_process.h
+++ b/src/qemu/qemu_process.h
@@ -94,6 +94,13 @@ int qemuProcessLaunch(virConnectPtr conn,
   virNetDevVPortProfileOp vmop,
   unsigned int flags);
 
+int qemuProcessFinishStartup(virConnectPtr conn,
+ virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ qemuDomainAsyncJob asyncJob,
+ bool sta

[libvirt] [PATCH v3 4/7] qemu: Separate incoming URI generation from qemuMigrationPrepareAny

[Jiri Denemark]

Signed-off-by: Jiri Denemark 
---

Notes:
ACKed in version 1

Version 3:
- no change

Version 2:
- no change

 src/qemu/qemu_migration.c | 154 +-
 1 file changed, 85 insertions(+), 69 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 0c4c94a..4ab6ab7 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3291,6 +3291,80 @@ qemuMigrationPrepareCleanup(virQEMUDriverPtr driver,
 qemuDomainObjDiscardAsyncJob(driver, vm);
 }
 
+static char *
+qemuMigrationPrepareIncoming(virDomainObjPtr vm,
+ bool tunnel,
+ const char *protocol,
+ const char *listenAddress,
+ unsigned short port)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+char *migrateFrom = NULL;
+
+if (tunnel) {
+if (VIR_STRDUP(migrateFrom, "stdio") < 0)
+goto cleanup;
+} else {
+bool encloseAddress = false;
+bool hostIPv6Capable = false;
+bool qemuIPv6Capable = false;
+struct addrinfo *info = NULL;
+struct addrinfo hints = { .ai_flags = AI_ADDRCONFIG,
+  .ai_socktype = SOCK_STREAM };
+const char *incFormat;
+
+if (getaddrinfo("::", NULL, &hints, &info) == 0) {
+freeaddrinfo(info);
+hostIPv6Capable = true;
+}
+qemuIPv6Capable = virQEMUCapsGet(priv->qemuCaps,
+ QEMU_CAPS_IPV6_MIGRATION);
+
+if (listenAddress) {
+if (virSocketAddrNumericFamily(listenAddress) == AF_INET6) {
+if (!qemuIPv6Capable) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+   _("qemu isn't capable of IPv6"));
+goto cleanup;
+}
+if (!hostIPv6Capable) {
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
+   _("host isn't capable of IPv6"));
+goto cleanup;
+}
+/* IPv6 address must be escaped in brackets on the cmd line */
+encloseAddress = true;
+} else {
+/* listenAddress is a hostname or IPv4 */
+}
+} else if (qemuIPv6Capable && hostIPv6Capable) {
+/* Listen on :: instead of 0.0.0.0 if QEMU understands it
+ * and there is at least one IPv6 address configured
+ */
+listenAddress = "::";
+encloseAddress = true;
+} else {
+listenAddress = "0.0.0.0";
+}
+
+/* QEMU will be started with
+ *   -incoming protocol:[]:port,
+ *   -incoming protocol::port, or
+ *   -incoming protocol::port
+ */
+if (encloseAddress)
+incFormat = "%s:[%s]:%d";
+else
+incFormat = "%s:%s:%d";
+if (virAsprintf(&migrateFrom, incFormat,
+protocol, listenAddress, port) < 0)
+goto cleanup;
+}
+
+ cleanup:
+return migrateFrom;
+}
+
 static int
 qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 virConnectPtr dconn,
@@ -3397,75 +3471,6 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 }
 }
 
-if (tunnel) {
-/* QEMU will be started with -incoming stdio
- * (which qemu_command might convert to exec:cat or fd:n)
- */
-if (VIR_STRDUP(migrateFrom, "stdio") < 0)
-goto cleanup;
-} else {
-bool encloseAddress = false;
-bool hostIPv6Capable = false;
-bool qemuIPv6Capable = false;
-virQEMUCapsPtr qemuCaps = NULL;
-struct addrinfo *info = NULL;
-struct addrinfo hints = { .ai_flags = AI_ADDRCONFIG,
-  .ai_socktype = SOCK_STREAM };
-const char *incFormat;
-
-if (getaddrinfo("::", NULL, &hints, &info) == 0) {
-freeaddrinfo(info);
-hostIPv6Capable = true;
-}
-if (!(qemuCaps = virQEMUCapsCacheLookupCopy(driver->qemuCapsCache,
-(*def)->emulator,
-(*def)->os.machine)))
-goto cleanup;
-
-qemuIPv6Capable = virQEMUCapsGet(qemuCaps, QEMU_CAPS_IPV6_MIGRATION);
-virObjectUnref(qemuCaps);
-
-if (listenAddress) {
-if (virSocketAddrNumericFamily(listenAddress) == AF_INET6) {
-if (!qemuIPv6Capable) {
-virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
-   _("qemu isn't capable of IPv6"));
-goto cleanup;
-}
-if (!hostIPv6Capable) {
-virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED

[libvirt] [PATCH v3 1/7] qemu: Introduce qemuProcessInit

[Jiri Denemark]

qemuProcessStart is going to be split in three parts: qemuProcessInit,
qemuProcessLaunch, and qemuProcessFinish so that migration Prepare phase
can insert additional code in the process. qemuProcessStart will be a
small wrapper for all other callers.

qemuProcessInit prepares the domain up to the point when priv->qemuCaps
is initialized.

Signed-off-by: Jiri Denemark 
---

Notes:
Version 3:
- call qemuProcessStop internally if necessary instead of pushing
  this responsibility to the caller

Version 2:
- avoid calling qemuProcessStop when starting fails with
  "VM is already active"

 src/qemu/qemu_process.c | 133 
 src/qemu/qemu_process.h |   4 ++
 2 files changed, 92 insertions(+), 45 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 68542e7..83a17ce 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4476,6 +4476,90 @@ qemuProcessMakeDir(virQEMUDriverPtr driver,
 }
 
 
+/**
+ * qemuProcessInit:
+ *
+ * Prepares the domain up to the point when priv->qemuCaps is initialized. The
+ * function calls qemuProcessStop when needed.
+ *
+ * Returns 0 on success, -1 on error.
+ */
+int
+qemuProcessInit(virQEMUDriverPtr driver,
+virDomainObjPtr vm,
+bool migration)
+{
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+virCapsPtr caps = NULL;
+qemuDomainObjPrivatePtr priv = vm->privateData;
+int stopFlags;
+int ret = -1;
+
+VIR_DEBUG("vm=%p name=%s id=%d migration=%d",
+  vm, vm->def->name, vm->def->id, migration);
+
+VIR_DEBUG("Beginning VM startup process");
+
+if (virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("VM is already active"));
+goto cleanup;
+}
+
+if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+goto stop;
+
+/* Some things, paths, ... are generated here and we want them to persist.
+ * Fill them in prior to setting the domain def as transient. */
+VIR_DEBUG("Generating paths");
+
+if (qemuPrepareNVRAM(cfg, vm, migration) < 0)
+goto stop;
+
+/* Do this upfront, so any part of the startup process can add
+ * runtime state to vm->def that won't be persisted. This let's us
+ * report implicit runtime defaults in the XML, like vnc listen/socket
+ */
+VIR_DEBUG("Setting current domain def as transient");
+if (virDomainObjSetDefTransient(caps, driver->xmlopt, vm, true) < 0)
+goto stop;
+
+vm->def->id = qemuDriverAllocateID(driver);
+qemuDomainSetFakeReboot(driver, vm, false);
+virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_STARTING_UP);
+
+if (virAtomicIntInc(&driver->nactive) == 1 && driver->inhibitCallback)
+driver->inhibitCallback(true, driver->inhibitOpaque);
+
+/* Run an early hook to set-up missing devices */
+if (qemuProcessStartHook(driver, vm,
+ VIR_HOOK_QEMU_OP_PREPARE,
+ VIR_HOOK_SUBOP_BEGIN) < 0)
+goto stop;
+
+VIR_DEBUG("Determining emulator version");
+virObjectUnref(priv->qemuCaps);
+if (!(priv->qemuCaps = virQEMUCapsCacheLookupCopy(driver->qemuCapsCache,
+  vm->def->emulator,
+  vm->def->os.machine)))
+goto stop;
+
+ret = 0;
+
+ cleanup:
+virObjectUnref(cfg);
+virObjectUnref(caps);
+return ret;
+
+ stop:
+stopFlags = VIR_QEMU_PROCESS_STOP_NO_RELABEL;
+if (migration)
+stopFlags |= VIR_QEMU_PROCESS_STOP_MIGRATED;
+qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, stopFlags);
+goto cleanup;
+}
+
+
 int qemuProcessStart(virConnectPtr conn,
  virQEMUDriverPtr driver,
  virDomainObjPtr vm,
@@ -4498,7 +4582,7 @@ int qemuProcessStart(virConnectPtr conn,
 size_t i;
 char *nodeset = NULL;
 unsigned int stop_flags;
-virQEMUDriverConfigPtr cfg;
+virQEMUDriverConfigPtr cfg = NULL;
 virCapsPtr caps = NULL;
 unsigned int hostdev_flags = 0;
 size_t nnicindexes = 0;
@@ -4516,6 +4600,9 @@ int qemuProcessStart(virConnectPtr conn,
   VIR_QEMU_PROCESS_START_PAUSED |
   VIR_QEMU_PROCESS_START_AUTODESTROY, -1);
 
+if (qemuProcessInit(driver, vm, !!migrateFrom))
+goto cleanup;
+
 cfg = virQEMUDriverGetConfig(driver);
 
 /* From now on until domain security labeling is done:
@@ -4534,53 +4621,9 @@ int qemuProcessStart(virConnectPtr conn,
 /* We don't increase cfg's reference counter here. */
 hookData.cfg = cfg;
 
-VIR_DEBUG("Beginning VM startup process");
-
-if (virDomainObjIsActive(vm)) {
-virReportError(VIR_ERR_OPERATION_INVALID,
-   "%s", _("VM is already active"));
-virObjectUnref(cfg);
-retur

[libvirt] [PATCH v3 0/7] qemu: Add support for -incoming defer

[Jiri Denemark]

Traditionally, we pass incoming migration URI on QEMU command line,
which has some drawbacks. Depending on the URI QEMU may initialize its
migration state immediately without giving us a chance to set any
additional migration parameters (this applies mainly for fd: URIs). For
some URIs the monitor may be completely blocked from the beginning until
migration is finished, which means we may be stuck in qmp_capabilities
command without being able to send any QMP commands.

QEMU solved this by introducing "defer" parameter for -incoming command
line option. This will tell QEMU to prepare for an incoming migration
while the actual incoming URI is sent using migrate-incoming QMP
command. Before calling this command we can normally talk to the
monitor and even set any migration parameters which will be honored by
the incoming migration.

Jiri Denemark (7):
  qemu: Introduce qemuProcessInit
  qemu: Introduce qemuProcessLaunch
  qemu: Introduce qemuProcessFinishStartup
  qemu: Separate incoming URI generation from qemuMigrationPrepareAny
  qemu: Kill QEMU process if Prepare phase fails
  qemu: Skip starting NBD servers for offline migration
  qemu: Use qemuProcessLaunch in migration Prepare phase

 src/qemu/qemu_migration.c | 247 +--
 src/qemu/qemu_process.c   | 489 +-
 src/qemu/qemu_process.h   |  20 ++
 3 files changed, 466 insertions(+), 290 deletions(-)

-- 
2.6.3

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

[libvirt] [PATCH v3 7/7] qemu: Use qemuProcessLaunch in migration Prepare phase

[Jiri Denemark]

Using qemuProcess{Init,Launch,FinishStartup} allows us to run
pre-migration commands on destination before asking QEMU to wait for
incoming migration data.

Signed-off-by: Jiri Denemark 
---

Notes:
Version 3:
- adapt to changes in 1/7 and 3/7

Version 2:
- adapt to changes in other patches

 src/qemu/qemu_migration.c | 65 ++-
 1 file changed, 41 insertions(+), 24 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index b6525df..9148a1e 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3291,14 +3291,16 @@ qemuMigrationPrepareCleanup(virQEMUDriverPtr driver,
 qemuDomainObjDiscardAsyncJob(driver, vm);
 }
 
-static char *
+static qemuProcessIncomingDefPtr
 qemuMigrationPrepareIncoming(virDomainObjPtr vm,
  bool tunnel,
  const char *protocol,
  const char *listenAddress,
- unsigned short port)
+ unsigned short port,
+ int fd)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
+qemuProcessIncomingDefPtr inc = NULL;
 char *migrateFrom = NULL;
 
 if (tunnel) {
@@ -3361,8 +3363,11 @@ qemuMigrationPrepareIncoming(virDomainObjPtr vm,
 goto cleanup;
 }
 
+inc = qemuProcessIncomingDefNew(priv->qemuCaps, migrateFrom, fd, NULL);
+
  cleanup:
-return migrateFrom;
+VIR_FREE(migrateFrom);
+return inc;
 }
 
 static int
@@ -3393,8 +3398,11 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 char *xmlout = NULL;
 unsigned int cookieFlags;
 virCapsPtr caps = NULL;
-char *migrateFrom = NULL;
+qemuProcessIncomingDefPtr incoming = NULL;
 bool taint_hook = false;
+bool stopProcess = false;
+bool relabel = false;
+int rv;
 
 virNWFilterReadLockFilterUpdates();
 
@@ -3528,28 +3536,26 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 goto stopjob;
 }
 
-virObjectUnref(priv->qemuCaps);
-priv->qemuCaps = virQEMUCapsCacheLookupCopy(driver->qemuCapsCache,
-vm->def->emulator,
-vm->def->os.machine);
-if (!priv->qemuCaps)
+if (qemuProcessInit(driver, vm, true) < 0)
 goto stopjob;
+stopProcess = true;
 
-if (!(migrateFrom = qemuMigrationPrepareIncoming(vm, tunnel, protocol,
- listenAddress, port)))
+if (!(incoming = qemuMigrationPrepareIncoming(vm, tunnel, protocol,
+  listenAddress, port,
+  dataFD[0])))
 goto stopjob;
+dataFD[0] = -1; /* the FD is now owned by incoming */
 
-/* Start the QEMU daemon, with the same command-line arguments plus
- * -incoming $migrateFrom
- */
-if (qemuProcessStart(dconn, driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN,
- migrateFrom, dataFD[0], NULL, NULL,
- VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START,
- VIR_QEMU_PROCESS_START_PAUSED |
- VIR_QEMU_PROCESS_START_AUTODESTROY) < 0) {
-virDomainAuditStart(vm, "migrated", false);
+rv = qemuProcessLaunch(dconn, driver, vm, QEMU_ASYNC_JOB_MIGRATION_IN,
+   incoming, NULL,
+   VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START,
+   VIR_QEMU_PROCESS_START_AUTODESTROY);
+if (rv < 0) {
+if (rv == -2)
+relabel = true;
 goto stopjob;
 }
+relabel = true;
 
 if (tunnel) {
 if (virFDStreamOpen(st, dataFD[1]) < 0) {
@@ -3594,6 +3600,15 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 VIR_DEBUG("Received no lockstate");
 }
 
+if (incoming->deferredURI &&
+qemuMigrationRunIncoming(driver, vm, incoming->deferredURI,
+ QEMU_ASYNC_JOB_MIGRATION_IN) < 0)
+goto stopjob;
+
+if (qemuProcessFinishStartup(dconn, driver, vm, 
QEMU_ASYNC_JOB_MIGRATION_IN,
+ false, VIR_DOMAIN_PAUSED_MIGRATION) < 0)
+goto stopjob;
+
  done:
 if (qemuMigrationBakeCookie(mig, driver, vm, cookieout,
 cookieoutlen, cookieFlags) < 0) {
@@ -3625,7 +3640,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 ret = 0;
 
  cleanup:
-VIR_FREE(migrateFrom);
+qemuProcessIncomingDefFree(incoming);
 VIR_FREE(xmlout);
 VIR_FORCE_CLOSE(dataFD[0]);
 VIR_FORCE_CLOSE(dataFD[1]);
@@ -3645,10 +3660,12 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 return ret;
 
  stopjob:
-if (vm->def->id != -1) {
+if (stopProcess) {
+unsigned int stopFlags = VIR_QEMU_PROCESS_STOP_MIGRATED;
+if (!relabel)
+stopFlags |= VIR_QE

[libvirt] [PATCH v3 5/7] qemu: Kill QEMU process if Prepare phase fails

[Jiri Denemark]

Some failure paths in qemuMigrationPrepareAny forgot to kill the just
started QEMU process. This patch fixes this by combining 'stop' and
'endjob' label into a new label 'stopjob'. This name was chosen to avoid
confusion with the most common semantics of 'endjob'. Normally, 'endjob'
is always called at the end of an API to stop the job we entered at the
beginning. In qemuMigrationPrepareAny we only want to stop the job in
failure path; on success we need to carry the job over to the Finish
phase.

Signed-off-by: Jiri Denemark 
---

Notes:
ACKed in version 1

Version 3:
- no change

Version 2:
- no change

 src/qemu/qemu_migration.c | 31 ---
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 4ab6ab7..9a4f19f 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -3522,7 +3522,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 (pipe(dataFD) < 0 || virSetCloseExec(dataFD[1]) < 0)) {
 virReportSystemError(errno, "%s",
  _("cannot create pipe for tunnelled migration"));
-goto endjob;
+goto stopjob;
 }
 
 virObjectUnref(priv->qemuCaps);
@@ -3530,11 +3530,11 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 vm->def->emulator,
 vm->def->os.machine);
 if (!priv->qemuCaps)
-goto endjob;
+goto stopjob;
 
 if (!(migrateFrom = qemuMigrationPrepareIncoming(vm, tunnel, protocol,
  listenAddress, port)))
-goto endjob;
+goto stopjob;
 
 /* Start the QEMU daemon, with the same command-line arguments plus
  * -incoming $migrateFrom
@@ -3545,14 +3545,14 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
  VIR_QEMU_PROCESS_START_PAUSED |
  VIR_QEMU_PROCESS_START_AUTODESTROY) < 0) {
 virDomainAuditStart(vm, "migrated", false);
-goto endjob;
+goto stopjob;
 }
 
 if (tunnel) {
 if (virFDStreamOpen(st, dataFD[1]) < 0) {
 virReportSystemError(errno, "%s",
  _("cannot pass pipe for tunnelled 
migration"));
-goto stop;
+goto stopjob;
 }
 dataFD[1] = -1; /* 'st' owns the FD now & will close it */
 }
@@ -3560,17 +3560,17 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 if (qemuMigrationSetCompression(driver, vm,
 flags & VIR_MIGRATE_COMPRESSED,
 QEMU_ASYNC_JOB_MIGRATION_IN) < 0)
-goto stop;
+goto stopjob;
 
 if (STREQ_NULLABLE(protocol, "rdma") &&
 virProcessSetMaxMemLock(vm->pid, vm->def->mem.hard_limit << 10) < 0) {
-goto stop;
+goto stopjob;
 }
 
 if (qemuMigrationSetPinAll(driver, vm,
flags & VIR_MIGRATE_RDMA_PIN_ALL,
QEMU_ASYNC_JOB_MIGRATION_IN) < 0)
-goto stop;
+goto stopjob;
 
 if (mig->lockState) {
 VIR_DEBUG("Received lockstate %s", mig->lockState);
@@ -3593,7 +3593,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 if (qemuMigrationStartNBDServer(driver, vm, listenAddress,
 nmigrate_disks, migrate_disks) < 0) {
 /* error already reported */
-goto endjob;
+goto stopjob;
 }
 cookieFlags |= QEMU_MIGRATION_COOKIE_NBD;
 }
@@ -3608,7 +3608,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 }
 
 if (qemuDomainCleanupAdd(vm, qemuMigrationPrepareCleanup) < 0)
-goto endjob;
+goto stopjob;
 
 if (!(flags & VIR_MIGRATE_OFFLINE)) {
 virDomainAuditStart(vm, "migrated", true);
@@ -3647,12 +3647,13 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
 virNWFilterUnlockFilterUpdates();
 return ret;
 
- stop:
-virDomainAuditStart(vm, "migrated", false);
-qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED,
-VIR_QEMU_PROCESS_STOP_MIGRATED);
+ stopjob:
+if (vm->def->id != -1) {
+virDomainAuditStart(vm, "migrated", false);
+qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED,
+VIR_QEMU_PROCESS_STOP_MIGRATED);
+}
 
- endjob:
 qemuMigrationJobFinish(driver, vm);
 goto cleanup;
 }
-- 
2.6.3

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

[libvirt] [PATCH v3 2/7] qemu: Introduce qemuProcessLaunch

[Jiri Denemark]

Once qemuProcessInit was called, qemuProcessLaunch will launch a new
QEMU process with stopped virtual CPUs.

Signed-off-by: Jiri Denemark 
---

Notes:
ACKed in version 2

Version 3:
- no change

Version 2:
- save status before running VIR_HOOK_QEMU_OP_STARTED hook
- call virSecurityManagerSetAllLabel even if !incoming
- move qemuProcessStop out of qemuProcessLaunch to avoid it being called 
twice

 src/qemu/qemu_process.c | 315 
 src/qemu/qemu_process.h |   9 ++
 2 files changed, 195 insertions(+), 129 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 83a17ce..1b3cbc0 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4560,16 +4560,28 @@ qemuProcessInit(virQEMUDriverPtr driver,
 }
 
 
-int qemuProcessStart(virConnectPtr conn,
- virQEMUDriverPtr driver,
- virDomainObjPtr vm,
- qemuDomainAsyncJob asyncJob,
- const char *migrateFrom,
- int migrateFd,
- const char *migratePath,
- virDomainSnapshotObjPtr snapshot,
- virNetDevVPortProfileOp vmop,
- unsigned int flags)
+/**
+ * qemuProcessLaunch:
+ *
+ * Launch a new QEMU process with stopped virtual CPUs.
+ *
+ * The caller is supposed to call qemuProcessStop with appropriate
+ * flags in case of failure.
+ *
+ * Returns 0 on success,
+ *-1 on error which happened before devices were labeled and thus
+ *   there is no need to restore them,
+ *-2 on error requesting security labels to be restored.
+ */
+int
+qemuProcessLaunch(virConnectPtr conn,
+  virQEMUDriverPtr driver,
+  virDomainObjPtr vm,
+  qemuDomainAsyncJob asyncJob,
+  qemuProcessIncomingDefPtr incoming,
+  virDomainSnapshotObjPtr snapshot,
+  virNetDevVPortProfileOp vmop,
+  unsigned int flags)
 {
 int ret = -1;
 int rv;
@@ -4581,18 +4593,22 @@ int qemuProcessStart(virConnectPtr conn,
 struct qemuProcessHookData hookData;
 size_t i;
 char *nodeset = NULL;
-unsigned int stop_flags;
-virQEMUDriverConfigPtr cfg = NULL;
+virQEMUDriverConfigPtr cfg;
 virCapsPtr caps = NULL;
 unsigned int hostdev_flags = 0;
 size_t nnicindexes = 0;
 int *nicindexes = NULL;
-qemuProcessIncomingDefPtr incoming = NULL;
 
-VIR_DEBUG("vm=%p name=%s id=%d asyncJob=%d migrateFrom=%s migrateFd=%d "
-  "migratePath=%s snapshot=%p vmop=%d flags=0x%x",
-  vm, vm->def->name, vm->def->id, asyncJob, NULLSTR(migrateFrom),
-  migrateFd, NULLSTR(migratePath), snapshot, vmop, flags);
+VIR_DEBUG("vm=%p name=%s id=%d asyncJob=%d "
+  "incoming.launchURI=%s incoming.deferredURI=%s "
+  "incoming.fd=%d incoming.path=%s "
+  "snapshot=%p vmop=%d flags=0x%x",
+  vm, vm->def->name, vm->def->id, asyncJob,
+  NULLSTR(incoming ? incoming->launchURI : NULL),
+  NULLSTR(incoming ? incoming->deferredURI : NULL),
+  incoming ? incoming->fd : -1,
+  NULLSTR(incoming ? incoming->path : NULL),
+  snapshot, vmop, flags);
 
 /* Okay, these are just internal flags,
  * but doesn't hurt to check */
@@ -4600,21 +4616,8 @@ int qemuProcessStart(virConnectPtr conn,
   VIR_QEMU_PROCESS_START_PAUSED |
   VIR_QEMU_PROCESS_START_AUTODESTROY, -1);
 
-if (qemuProcessInit(driver, vm, !!migrateFrom))
-goto cleanup;
-
 cfg = virQEMUDriverGetConfig(driver);
 
-/* From now on until domain security labeling is done:
- * if any operation fails and we goto cleanup, we must not
- * restore any security label as we would overwrite labels
- * we did not set. */
-stop_flags = VIR_QEMU_PROCESS_STOP_NO_RELABEL;
-/* If we fail while doing incoming migration, then we must not
- * relabel, as the source is still using the files.  */
-if (migrateFrom)
-stop_flags |= VIR_QEMU_PROCESS_STOP_MIGRATED;
-
 hookData.conn = conn;
 hookData.vm = vm;
 hookData.driver = driver;
@@ -4622,7 +4625,7 @@ int qemuProcessStart(virConnectPtr conn,
 hookData.cfg = cfg;
 
 if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
-goto error;
+goto cleanup;
 
 /* network devices must be "prepared" before hostdevs, because
  * setting up a network device might create a new hostdev that
@@ -4630,35 +4633,35 @@ int qemuProcessStart(virConnectPtr conn,
  */
 VIR_DEBUG("Preparing network devices");
 if (qemuNetworkPrepareDevices(vm->def) < 0)
-goto error;
+goto cleanup;
 
 /* Must be run before security labelling */
 VIR_DEBUG("Preparing host devices");
 if (!cfg->relaxedACS)
 hostdev

Re: [libvirt] ANNOUNCE: virt-manager 1.3.0 released

[Martin Kletzander]

On Tue, Nov 24, 2015 at 10:11:04PM -0500, Cole Robinson wrote:

I'm happy to announce the release of virt-manager 1.3.0!



Yay! \o/  So are we synchronizing virt-manager and libvirt version
numbers now? =)


virt-manager is a desktop application for managing KVM, Xen, and LXC
virtualization via libvirt.

The release can be downloaded from:

http://virt-manager.org/download/

This release includes:

- Git hosting moved to http://github.com/virt-manager/virt-manager
- Switch translation infrastructure from transifex to fedora.zanata.org
- Add dogtail UI tests and infrastructure
- Improved support for s390x kvm (Kevin Zhao)
- virt-install and virt-manager now remove created disk images if VM
 install startup fails
- Replace urlgrabber usage with requests and urllib2
- virt-install: add --network virtualport support for openvswitch
 (Daniel P. Berrange)
- virt-install: support multiple --security labels
- virt-install: support --features kvm_hidden=on|off (Pavel Hrdina)
- virt-install: add --features pmu=on|off
- virt-install: add --features pvspinlock=on|off (Abhijeet Kasurde)
- virt-install: add --events on_lockfailure=on|off (Abhijeet Kasurde)
- virt-install: add --network link_state=up|down
- virt-install: add --vcpu placement=static|auto

Thanks to everyone who has contributed to this release through testing,
bug reporting, submitting patches, and otherwise sending in feedback!

Thanks,
Cole

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


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

[libvirt] [PATCH v2 3/5] qemu: add virtio video device

[Marc-André Lureau]

qemu 2.5 provides virtio video device.  It can be used with -device
virtio-vga for primary devices, or -device virtio-gpu for non-vga
devices. However, only the primary device (VGA) is supported with this
patch.

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

Signed-off-by: Marc-André Lureau 
---
 docs/formatdomain.html.in  |5 +-
 docs/schemas/domaincommon.rng  |1 +
 src/conf/domain_conf.c |3 +-
 src/conf/domain_conf.h |1 +
 src/qemu/qemu_capabilities.c   |3 +
 src/qemu/qemu_capabilities.h   |1 +
 src/qemu/qemu_command.c|   12 +-
 tests/qemucapabilitiesdata/caps_2.4.0-1.caps   |1 +
 tests/qemucapabilitiesdata/caps_2.5.0-1.caps   |  166 +
 tests/qemucapabilitiesdata/caps_2.5.0-1.replies| 3953 
 tests/qemucapabilitiestest.c   |1 +
 .../qemuxml2argv-video-virtio-gpu-device.args  |   23 +
 .../qemuxml2argv-video-virtio-gpu-device.xml   |   31 +
 tests/qemuxml2argvtest.c   |4 +
 tests/qemuxml2xmltest.c|2 +
 15 files changed, 4200 insertions(+), 7 deletions(-)
 create mode 100644 tests/qemucapabilitiesdata/caps_2.5.0-1.caps
 create mode 100644 tests/qemucapabilitiesdata/caps_2.5.0-1.replies
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-device.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-device.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index e5e0167..23d8ac9 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -5153,8 +5153,9 @@ qemu-kvm -net nic,model=? /dev/null
 
   The model element has a mandatory type
   attribute which takes the value "vga", "cirrus", "vmvga", "xen",
-  "vbox", or "qxl" (since 0.8.6) depending
-  on the hypervisor features available.
+  "vbox", "qxl" (since 0.8.6) or
+  "virtio" (since 1.3.0)
+  depending on the hypervisor features available.
 
 
   You can provide the amount of video memory in kibibytes (blocks of
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 994face..228f062 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2921,6 +2921,7 @@
 vmvga
 xen
 vbox
+virtio
   
 
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 111c2ae..e1692ef 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -532,7 +532,8 @@ VIR_ENUM_IMPL(virDomainVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,
   "xen",
   "vbox",
   "qxl",
-  "parallels")
+  "parallels",
+  "virtio")
 
 VIR_ENUM_IMPL(virDomainInput, VIR_DOMAIN_INPUT_TYPE_LAST,
   "mouse",
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a1a9b48..a47d8ee 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1374,6 +1374,7 @@ typedef enum {
 VIR_DOMAIN_VIDEO_TYPE_VBOX,
 VIR_DOMAIN_VIDEO_TYPE_QXL,
 VIR_DOMAIN_VIDEO_TYPE_PARALLELS, /* pseudo device for VNC in containers */
+VIR_DOMAIN_VIDEO_TYPE_VIRTIO,
 
 VIR_DOMAIN_VIDEO_TYPE_LAST
 } virDomainVideoType;
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 2813212..12e2098 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -301,6 +301,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "gic-version",
 
   "incoming-defer", /* 200 */
+  "virtio-gpu",
 );
 
 
@@ -1543,6 +1544,8 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
 { "virtio-net-ccw", QEMU_CAPS_DEVICE_VIRTIO_NET },
 { "virtio-net-s390", QEMU_CAPS_DEVICE_VIRTIO_NET },
 { "virtio-net-device", QEMU_CAPS_DEVICE_VIRTIO_NET },
+{ "virtio-gpu-pci", QEMU_CAPS_DEVICE_VIRTIO_GPU },
+{ "virtio-gpu-device", QEMU_CAPS_DEVICE_VIRTIO_GPU },
 };
 
 static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index e3e40e5..8a7c97e 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -327,6 +327,7 @@ typedef enum {
 
 /* 200 */
 QEMU_CAPS_INCOMING_DEFER, /* -incoming defer and migrate_incoming */
+QEMU_CAPS_DEVICE_VIRTIO_GPU, /* -device virtio-gpu-* & virtio-vga */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4d00fd9..887c87e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -104,7 +104,8 @@ VIR_ENUM_IMPL(qemuVideo, VIR_DOMAIN_VIDEO_TYPE_LAST,

[libvirt] [PATCH v2 5/5] RFC qemu: add spice opengl support

[Marc-André Lureau]

Add Spice graphics gl attribute. qemu 2.6 should have -spice gl=on
argument to enable opengl rendering context. This is necessary
to actually enable virgl rendering.

Add a qemuxml2argv test for virtio-gpu + spice with virgl.

Signed-off-by: Marc-André Lureau 
---
 docs/formatdomain.html.in  |  6 
 docs/schemas/domaincommon.rng  |  5 +++
 src/conf/domain_conf.c | 18 +++
 src/conf/domain_conf.h |  1 +
 src/qemu/qemu_capabilities.c   |  2 ++
 src/qemu/qemu_capabilities.h   |  1 +
 src/qemu/qemu_command.c| 11 +++
 tests/qemucapabilitiesdata/caps_2.5.0-1.caps   |  1 +
 tests/qemucapabilitiesdata/caps_2.5.0-1.replies|  4 +++
 .../qemuxml2argv-video-virtio-gpu-spice-gl.args| 23 ++
 .../qemuxml2argv-video-virtio-gpu-spice-gl.xml | 36 ++
 tests/qemuxml2argvtest.c   |  6 
 tests/qemuxml2xmltest.c|  1 +
 13 files changed, 115 insertions(+)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 23d8ac9..d39f7d0 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -4979,6 +4979,12 @@ qemu-kvm -net nic,model=? /dev/null
   0.8.8); and usbredir
   (since 0.9.12).
 
+
+  Spice may provide accelerated server-side rendering with
+  OpenGL. You can enable or disable OpenGL support explicitly with
+  the gl attribute.
+  (since FIXME).
+
 
   
 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 228f062..8f4d2ac 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2711,6 +2711,11 @@
   
 
   
+  
+
+  
+
+  
   
 
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e1692ef..d738e71 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10918,6 +10918,7 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
 char *port = virXMLPropString(node, "port");
 char *tlsPort;
 char *autoport;
+char *gl;
 char *defaultMode;
 int defaultModeVal;
 
@@ -10952,6 +10953,19 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
 VIR_FREE(autoport);
 }
 
+if ((gl = virXMLPropString(node, "gl")) != NULL) {
+int glVal;
+
+if ((glVal = virTristateBoolTypeFromString(gl)) <= 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unknown gl value '%s'"), gl);
+goto error;
+}
+
+def->data.spice.gl = glVal;
+VIR_FREE(gl);
+}
+
 def->data.spice.defaultMode = 
VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_MODE_ANY;
 
 if ((defaultMode = virXMLPropString(node, "defaultMode")) != NULL) {
@@ -21201,6 +21215,10 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
 break;
 
 case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
+if (def->data.spice.gl)
+virBufferAsprintf(buf, " gl='%s'",
+  virTristateBoolTypeToString(def->data.spice.gl));
+
 if (def->data.spice.port)
 virBufferAsprintf(buf, " port='%d'",
   def->data.spice.port);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a47d8ee..ccd9376 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1579,6 +1579,7 @@ struct _virDomainGraphicsDef {
 int streaming;
 int copypaste; /* enum virTristateBool */
 int filetransfer; /* enum virTristateBool */
+int gl; /* enum virTristateBool */
 } spice;
 } data;
 /* nListens, listens, and *port are only useful if type is vnc,
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 4d76c40..64804c8 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -303,6 +303,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "incoming-defer", /* 200 */
   "virtio-gpu",
   "virtio-gpu.virgl",
+  "spice-gl",
 );
 
 
@@ -2587,6 +2588,7 @@ static struct virQEMUCapsCommandLineProps 
virQEMUCapsCommandLine[] = {
 { "drive", "throttling.bps-total-max", QEMU_CAPS_DRIVE_IOTUNE_MAX},
 { "machine", "aes-key-wrap", QEMU_CAPS_AES_KEY_WRAP },
 { "machine", "dea-key-wrap", QEMU_CAPS_DEA_KEY_WRAP 

[libvirt] [PATCH v2 1/5] Replace support{2d,3d} with accel{2d,3d}

[Marc-André Lureau]

Following the domain XML naming

Signed-off-by: Marc-André Lureau 
---
 src/conf/domain_conf.c | 44 ++--
 src/conf/domain_conf.h |  4 ++--
 src/vbox/vbox_common.c |  8 
 src/vz/vz_sdk.c|  2 +-
 4 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 616bf80..f501f14 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11961,41 +11961,41 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
 {
 xmlNodePtr cur;
 virDomainVideoAccelDefPtr def;
-char *support3d = NULL;
-char *support2d = NULL;
+char *accel3d = NULL;
+char *accel2d = NULL;
 
 cur = node->children;
 while (cur != NULL) {
 if (cur->type == XML_ELEMENT_NODE) {
-if (!support3d && !support2d &&
+if (!accel3d && !accel2d &&
 xmlStrEqual(cur->name, BAD_CAST "acceleration")) {
-support3d = virXMLPropString(cur, "accel3d");
-support2d = virXMLPropString(cur, "accel2d");
+accel3d = virXMLPropString(cur, "accel3d");
+accel2d = virXMLPropString(cur, "accel2d");
 }
 }
 cur = cur->next;
 }
 
-if (!support3d && !support2d)
+if (!accel3d && !accel2d)
 return NULL;
 
 if (VIR_ALLOC(def) < 0)
 return NULL;
 
-if (support3d) {
-if (STREQ(support3d, "yes"))
-def->support3d = true;
+if (accel3d) {
+if (STREQ(accel3d, "yes"))
+def->accel3d = true;
 else
-def->support3d = false;
-VIR_FREE(support3d);
+def->accel3d = false;
+VIR_FREE(accel3d);
 }
 
-if (support2d) {
-if (STREQ(support2d, "yes"))
-def->support2d = true;
+if (accel2d) {
+if (STREQ(accel2d, "yes"))
+def->accel2d = true;
 else
-def->support2d = false;
-VIR_FREE(support2d);
+def->accel2d = false;
+VIR_FREE(accel2d);
 }
 
 return def;
@@ -17170,17 +17170,17 @@ 
virDomainVideoDefCheckABIStability(virDomainVideoDefPtr src,
 }
 
 if (src->accel) {
-if (src->accel->support2d != dst->accel->support2d) {
+if (src->accel->accel2d != dst->accel->accel2d) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Target video card 2d accel %u does not match 
source %u"),
-   dst->accel->support2d, src->accel->support2d);
+   dst->accel->accel2d, src->accel->accel2d);
 return false;
 }
 
-if (src->accel->support3d != dst->accel->support3d) {
+if (src->accel->accel3d != dst->accel->accel3d) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Target video card 3d accel %u does not match 
source %u"),
-   dst->accel->support3d, src->accel->support3d);
+   dst->accel->accel3d, src->accel->accel3d);
 return false;
 }
 }
@@ -20838,9 +20838,9 @@ virDomainVideoAccelDefFormat(virBufferPtr buf,
  virDomainVideoAccelDefPtr def)
 {
 virBufferAsprintf(buf, "support3d ? "yes" : "no");
+  def->accel3d ? "yes" : "no");
 virBufferAsprintf(buf, " accel2d='%s'",
-  def->support2d ? "yes" : "no");
+  def->accel2d ? "yes" : "no");
 virBufferAddLit(buf, "/>\n");
 }
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 8d43ee6..40ad68c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1382,8 +1382,8 @@ typedef enum {
 typedef struct _virDomainVideoAccelDef virDomainVideoAccelDef;
 typedef virDomainVideoAccelDef *virDomainVideoAccelDefPtr;
 struct _virDomainVideoAccelDef {
-bool support3d;
-bool support2d;
+bool accel3d;
+bool accel2d;
 };
 
 
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 3e6ed7a..4839a62 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -1554,10 +1554,10 @@ vboxAttachVideo(virDomainDefPtr def, IMachine *machine)
 gVBoxAPI.UIMachine.SetMonitorCount(machine, def->videos[0]->heads);
 if (def->videos[0]->accel) {
 gVBoxAPI.UIMachine.SetAccelerate3DEnabled(machine,
-  
def->videos[0]->accel->support3d);
+  
def->videos[0]->accel->accel3d);
 if (gVBoxAPI.accelerate2DVideo)
 gVBoxAPI.UIMachine.SetAccelerate2DVideoEnabled(machine,
-   
def->videos[0]->accel->support2d);
+   
def->videos[0]->accel->accel2d);
 } else {
 gVBoxAPI.UIMachine.SetAccelerate3DEnabled(machine, 0);
   

[libvirt] [PATCH v2 2/5] domain: replace bool accel{2d, 3d} with a tristate

[Marc-André Lureau]

Allowing to have the extra undefined/default state.

Signed-off-by: Marc-André Lureau 
---
 src/conf/domain_conf.c | 41 ++---
 src/conf/domain_conf.h |  4 ++--
 src/vbox/vbox_common.c | 18 --
 3 files changed, 40 insertions(+), 23 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f501f14..111c2ae 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11961,8 +11961,9 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
 {
 xmlNodePtr cur;
 virDomainVideoAccelDefPtr def;
-char *accel3d = NULL;
 char *accel2d = NULL;
+char *accel3d = NULL;
+int val;
 
 cur = node->children;
 while (cur != NULL) {
@@ -11983,21 +11984,26 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
 return NULL;
 
 if (accel3d) {
-if (STREQ(accel3d, "yes"))
-def->accel3d = true;
-else
-def->accel3d = false;
-VIR_FREE(accel3d);
+if ((val = virTristateBoolTypeFromString(accel3d)) <= 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unknown accel3d value '%s'"), accel3d);
+goto end;
+}
+def->accel3d = val;
 }
 
 if (accel2d) {
-if (STREQ(accel2d, "yes"))
-def->accel2d = true;
-else
-def->accel2d = false;
-VIR_FREE(accel2d);
+if ((val = virTristateBoolTypeFromString(accel2d)) <= 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unknown accel2d value '%s'"), accel2d);
+goto end;
+}
+def->accel2d = val;
 }
 
+end:
+VIR_FREE(accel2d);
+VIR_FREE(accel3d);
 return def;
 }
 
@@ -20837,10 +20843,15 @@ static void
 virDomainVideoAccelDefFormat(virBufferPtr buf,
  virDomainVideoAccelDefPtr def)
 {
-virBufferAsprintf(buf, "accel3d ? "yes" : "no");
-virBufferAsprintf(buf, " accel2d='%s'",
-  def->accel2d ? "yes" : "no");
+virBufferAsprintf(buf, "accel3d) {
+virBufferAsprintf(buf, " accel3d='%s'",
+  virTristateBoolTypeToString(def->accel3d));
+}
+if (def->accel2d) {
+virBufferAsprintf(buf, " accel2d='%s'",
+  virTristateBoolTypeToString(def->accel2d));
+}
 virBufferAddLit(buf, "/>\n");
 }
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 40ad68c..a1a9b48 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1382,8 +1382,8 @@ typedef enum {
 typedef struct _virDomainVideoAccelDef virDomainVideoAccelDef;
 typedef virDomainVideoAccelDef *virDomainVideoAccelDefPtr;
 struct _virDomainVideoAccelDef {
-bool accel3d;
-bool accel2d;
+int accel2d; /* enum virTristateBool */
+int accel3d; /* enum virTristateBool */
 };
 
 
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 4839a62..0e84f30 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -1553,11 +1553,15 @@ vboxAttachVideo(virDomainDefPtr def, IMachine *machine)
VIR_DIV_UP(def->videos[0]->vram, 1024));
 gVBoxAPI.UIMachine.SetMonitorCount(machine, def->videos[0]->heads);
 if (def->videos[0]->accel) {
-gVBoxAPI.UIMachine.SetAccelerate3DEnabled(machine,
-  
def->videos[0]->accel->accel3d);
-if (gVBoxAPI.accelerate2DVideo)
+if (def->videos[0]->accel->accel3d) {
+gVBoxAPI.UIMachine.SetAccelerate3DEnabled(machine,
+def->videos[0]->accel->accel3d == VIR_TRISTATE_BOOL_YES);
+}
+if (def->videos[0]->accel->accel2d &&
+gVBoxAPI.accelerate2DVideo) {
 gVBoxAPI.UIMachine.SetAccelerate2DVideoEnabled(machine,
-   
def->videos[0]->accel->accel2d);
+def->videos[0]->accel->accel2d == VIR_TRISTATE_BOOL_YES);
+}
 } else {
 gVBoxAPI.UIMachine.SetAccelerate3DEnabled(machine, 0);
 if (gVBoxAPI.accelerate2DVideo)
@@ -3277,8 +3281,10 @@ vboxDumpVideo(virDomainDefPtr def, vboxGlobalData *data 
ATTRIBUTE_UNUSED,
 def->videos[0]->vram= VRAMSize * 1024;
 def->videos[0]->heads   = monitorCount;
 if (VIR_ALLOC(def->videos[0]->accel) >= 0) {
-def->videos[0]->accel->accel3d = accelerate3DEnabled;
-def->videos[0]->accel->accel2d = accelerate2DEnabled;
+def->videos[0]->accel->accel3d = accelerate3DEnabled ?
+VIR_TRISTATE_BOOL_YES : VIR_TRISTATE_BOOL_NO;
+def->videos[0]->accel->accel2d = accelerate2DEnabled ?
+VIR_TRISTATE_BOOL_YES : VIR_TRISTATE_BOOL_NO;
 }
 }
 }
-- 
2.5.0

--
libvir-li

[libvirt] [PATCH v2 4/5] qemu: add virtio-gpu virgl support

[Marc-André Lureau]

Check if virtio-gpu provides virgl option, and add qemu command line
formatter.

It is enabled with the existing accel3d attribute:

 


Signed-off-by: Marc-André Lureau 
---
 src/qemu/qemu_capabilities.c   |   7 ++
 src/qemu/qemu_capabilities.h   |   1 +
 src/qemu/qemu_command.c|  17 +++-
 tests/qemucapabilitiesdata/caps_1.2.2-1.replies|  22 +++--
 tests/qemucapabilitiesdata/caps_1.3.1-1.replies|  22 +++--
 tests/qemucapabilitiesdata/caps_1.4.2-1.replies|  22 +++--
 tests/qemucapabilitiesdata/caps_1.5.3-1.replies|  22 +++--
 tests/qemucapabilitiesdata/caps_1.6.0-1.replies|  22 +++--
 tests/qemucapabilitiesdata/caps_1.6.50-1.replies   |  22 +++--
 tests/qemucapabilitiesdata/caps_2.1.1-1.replies|  22 +++--
 tests/qemucapabilitiesdata/caps_2.4.0-1.replies|  95 ++--
 tests/qemucapabilitiesdata/caps_2.5.0-1.caps   |   1 +
 tests/qemucapabilitiesdata/caps_2.5.0-1.replies| 100 +++--
 .../qemuxml2argv-video-virtio-gpu-virgl.args   |  23 +
 .../qemuxml2argv-video-virtio-gpu-virgl.xml|  33 +++
 tests/qemuxml2argvtest.c   |   4 +
 tests/qemuxml2xmltest.c|   1 +
 17 files changed, 372 insertions(+), 64 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-virgl.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-virgl.xml

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 12e2098..4d76c40 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -302,6 +302,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
 
   "incoming-defer", /* 200 */
   "virtio-gpu",
+  "virtio-gpu.virgl",
 );
 
 
@@ -1631,6 +1632,10 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsObjectPropsQxlVga[] = {
 { "vgamem_mb", QEMU_CAPS_QXL_VGA_VGAMEM },
 };
 
+static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioGpu[] = {
+{ "virgl", QEMU_CAPS_DEVICE_VIRTIO_GPU_VIRGL },
+};
+
 struct virQEMUCapsObjectTypeProps {
 const char *type;
 struct virQEMUCapsStringFlags *props;
@@ -1684,6 +1689,8 @@ static struct virQEMUCapsObjectTypeProps 
virQEMUCapsObjectProps[] = {
   ARRAY_CARDINALITY(virQEMUCapsObjectPropsQxl) },
 { "qxl-vga", virQEMUCapsObjectPropsQxlVga,
   ARRAY_CARDINALITY(virQEMUCapsObjectPropsQxlVga) },
+{ "virtio-gpu-pci", virQEMUCapsObjectPropsVirtioGpu,
+  ARRAY_CARDINALITY(virQEMUCapsObjectPropsVirtioGpu) },
 };
 
 
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 8a7c97e..ab711ff 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -328,6 +328,7 @@ typedef enum {
 /* 200 */
 QEMU_CAPS_INCOMING_DEFER, /* -incoming defer and migrate_incoming */
 QEMU_CAPS_DEVICE_VIRTIO_GPU, /* -device virtio-gpu-* & virtio-vga */
+QEMU_CAPS_DEVICE_VIRTIO_GPU_VIRGL, /* -device virtio-gpu-*.virgl */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 887c87e..2808960 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5896,7 +5896,22 @@ qemuBuildDeviceVideoStr(virDomainDefPtr def,
 
 virBufferAsprintf(&buf, "%s,id=%s", model, video->info.alias);
 
-if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL) {
+if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO) {
+if (video->accel) {
+if (video->accel->accel3d &&
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU_VIRGL)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   "%s", _("virtio-gpu 3d acceleration is not 
supported"));
+goto error;
+}
+
+if (video->accel->accel3d) {
+virBufferAsprintf(&buf, ",virgl=%s",
+  
virTristateSwitchTypeToString(video->accel->accel3d));
+}
+}
+
+} else if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL) {
 if (video->vram > (UINT_MAX / 1024)) {
 virReportError(VIR_ERR_OVERFLOW,
_("value for 'vram' must be less than '%u'"),
diff --git a/tests/qemucapabilitiesdata/caps_1.2.2-1.replies 
b/tests/qemucapabilitiesdata/caps_1.2.2-1.replies
index f501218..e7de77b 100644
--- a/tests/qemucapabilitiesdata/caps_1.2.2-1.replies
+++ b/tests/qemucapabilitiesdata/caps_1.2.2-1.replies
@@ -1598,6 +1598,14 @@
 }
 
 {
+"id": "libvirt-31",
+"error": {
+"class": "DeviceNotFound",
+"desc": "Device 'virtio-gpu-pci' not found"
+}
+}
+
+{
 "return": [
 {
 "name": "xenpv"
@@ -1641,7 +1649,7 @@
 "name": "none"
 }
 ],
-"id": "libvirt-31"
+"id": "libvirt-32"
 }
 
 {
@@ -1713,7 +1721,7 @@
 "name": "O

[libvirt] [PATCH v2 0/5] Add virtio-gpu & virgl support

[Marc-André Lureau]

The following series allow using virtio-gpu and virgl if qemu support
it. Ths Spice bits are not yet in qemu 2.5, but should make it in 2.6
and I added the RFC patches that are expected to not change much.

v1->v2: addressing Ján Tomko review
- update accel2d/accel3d to be tristate and fix its usage
- use a single QEMU_CAPS_DEVICE_VIRTIO_GPU cap for -device
  virtio-gpu-* & virtio-vga
- remove qemu legacy, -vga support or command line parsing
- add xml2xml tests
- squash xml2argvtests with their respecitve feature addition commits
- update libvirt version in doc
- update commit messages

Marc-André Lureau (5):
  Replace support{2d,3d} with accel{2d,3d}
  domain: replace bool accel{2d,3d} with a tristate
  qemu: add virtio video device
  qemu: add virtio-gpu virgl support
  RFC qemu: add spice opengl support

 docs/formatdomain.html.in  |   11 +-
 docs/schemas/domaincommon.rng  |6 +
 src/conf/domain_conf.c |   84 +-
 src/conf/domain_conf.h |6 +-
 src/qemu/qemu_capabilities.c   |   12 +
 src/qemu/qemu_capabilities.h   |3 +
 src/qemu/qemu_command.c|   40 +-
 src/vbox/vbox_common.c |   18 +-
 src/vz/vz_sdk.c|2 +-
 tests/qemucapabilitiesdata/caps_1.2.2-1.replies|   22 +-
 tests/qemucapabilitiesdata/caps_1.3.1-1.replies|   22 +-
 tests/qemucapabilitiesdata/caps_1.4.2-1.replies|   22 +-
 tests/qemucapabilitiesdata/caps_1.5.3-1.replies|   22 +-
 tests/qemucapabilitiesdata/caps_1.6.0-1.replies|   22 +-
 tests/qemucapabilitiesdata/caps_1.6.50-1.replies   |   22 +-
 tests/qemucapabilitiesdata/caps_2.1.1-1.replies|   22 +-
 tests/qemucapabilitiesdata/caps_2.4.0-1.caps   |1 +
 tests/qemucapabilitiesdata/caps_2.4.0-1.replies|   95 +-
 tests/qemucapabilitiesdata/caps_2.5.0-1.caps   |  168 +
 tests/qemucapabilitiesdata/caps_2.5.0-1.replies| 4043 
 tests/qemucapabilitiestest.c   |1 +
 .../qemuxml2argv-video-virtio-gpu-device.args  |   23 +
 .../qemuxml2argv-video-virtio-gpu-device.xml   |   31 +
 .../qemuxml2argv-video-virtio-gpu-spice-gl.args|   23 +
 .../qemuxml2argv-video-virtio-gpu-spice-gl.xml |   36 +
 .../qemuxml2argv-video-virtio-gpu-virgl.args   |   23 +
 .../qemuxml2argv-video-virtio-gpu-virgl.xml|   33 +
 tests/qemuxml2argvtest.c   |   14 +
 tests/qemuxml2xmltest.c|4 +
 29 files changed, 4732 insertions(+), 99 deletions(-)
 create mode 100644 tests/qemucapabilitiesdata/caps_2.5.0-1.caps
 create mode 100644 tests/qemucapabilitiesdata/caps_2.5.0-1.replies
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-device.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-device.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-spice-gl.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-virgl.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-video-virtio-gpu-virgl.xml

-- 
2.5.0

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

Re: [libvirt] [PATCH 4/8] perf: implement the remote protocol for perf event

[Jiri Denemark]

On Wed, Nov 25, 2015 at 01:22:22 +, Ren, Qiaowei wrote:
> 
> > -Original Message-
> > From: Jiri Denemark [mailto:jdene...@redhat.com]
> > Sent: Tuesday, November 24, 2015 5:43 PM
> > To: Ren, Qiaowei
> > Cc: libvir-list@redhat.com
> > Subject: Re: [libvirt] [PATCH 4/8] perf: implement the remote protocol for 
> > perf
> > event
> > 
> > On Tue, Nov 17, 2015 at 16:00:44 +0800, Qiaowei Ren wrote:
> > > Add remote support for perf event.
> > >
> > > Signed-off-by: Qiaowei Ren 
> > > ---
> > >  daemon/remote.c  | 60
> > 
> > >  src/remote/remote_driver.c   | 49
> > 
> > >  src/remote/remote_protocol.x | 32 ++-
> > > src/remote_protocol-structs  | 20 +++
> > >  4 files changed, 160 insertions(+), 1 deletion(-)
> > 
> > This will need to be changed to match the new style of the API. But, since 
> > you
> > are adding a new API, would you mind creating a patch for libvirt-python to 
> > add
> > this new API to python bindings?
> > 
> 
> Sure. The patch for libvirt-python should be also sent this maillist with 
> this series, right?

Well, it's a different repository so sending it in the same series might
be a bit tricky. So it's up to you if you send it in one series or send
it as a separate patch. And yes, they should all be sent to this mailing
list.

Jirka

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

Re: [libvirt] [PATCH v2 5/7] qemu: Kill QEMU process if Prepare phase fails

[Jiri Denemark]

On Tue, Nov 24, 2015 at 10:39:20 +0100, Pavel Hrdina wrote:
> On Thu, Nov 19, 2015 at 01:09:19PM +0100, Jiri Denemark wrote:
> > Some failure paths in qemuMigrationPrepareAny forgot to kill the just
> > started QEMU process. This patch fixes this by combining 'stop' and
> > 'endjob' label into a new label 'stopjob'. This name was chosen to avoid
> > confusion with the most common semantics of 'endjob'. Normally, 'endjob'
> > is always called at the end of an API to stop the job we entered at the
> > beginning. In qemuMigrationPrepareAny we only want to stop the job in
> > failure path; on success we need to carry the job over to the Finish
> > phase.
> > 
> > Signed-off-by: Jiri Denemark 
> > ---
> > 
> > Notes:
> > ACKed in version 1
> > 
> > Version 2:
> > - no change
> > 
> >  src/qemu/qemu_migration.c | 31 ---
> >  1 file changed, 16 insertions(+), 15 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> > index 4ab6ab7..9a4f19f 100644
> > --- a/src/qemu/qemu_migration.c
> > +++ b/src/qemu/qemu_migration.c
...
> > @@ -3593,7 +3593,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
> >  if (qemuMigrationStartNBDServer(driver, vm, listenAddress,
> >  nmigrate_disks, migrate_disks) < 
> > 0) {
> >  /* error already reported */
> > -goto endjob;
> > +goto stopjob;
> >  }
> >  cookieFlags |= QEMU_MIGRATION_COOKIE_NBD;
> >  }
> > @@ -3608,7 +3608,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
> >  }
> >  
> >  if (qemuDomainCleanupAdd(vm, qemuMigrationPrepareCleanup) < 0)
> > -goto endjob;
> > +goto stopjob;
> >  
> >  if (!(flags & VIR_MIGRATE_OFFLINE)) {
> >  virDomainAuditStart(vm, "migrated", true);
...
> I wouldn't join those two labels together regarding the change in 1/7 patch.
> Just change all goto after qemuProcessStart to jump to stop.  About the naming
> we've discussed, what about endmigjob? It's maybe to long, but tels, that we 
> are
> about to stop the qemuMigrationJob.

The bug was in the two hunks above. There are two paths going through
the code there, one path would need to stop the job and the process
while the other path should only stop the job. So instead of having the
logic there, I merged the two labels and decide what to do in one place.

Jirka

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