Re: [libvirt] [PATCH v3 0/9] add virDomainGetGuestInfo()

[Daniel Henrique Barboza]

Tested the patch series with a ppc64 guest, using upstream QEMU
in a Power8 host:

$ sudo ./run tools/virsh guestinfo qga-test
user.count  : 1
user.0.name : danielhb
user.0.login-time   : 1566588366375
os.id   : ubuntu
os.name : Ubuntu
os.pretty-name  : Ubuntu 18.04.1 LTS
os.version  : 18.04.1 LTS (Bionic Beaver)
os.version-id   : 18.04
os.machine  : ppc64le
os.kernel-release   : 4.15.0-29-generic
os.kernel-version   : #31-Ubuntu SMP Tue Jul 17 15:37:15 UTC 2018
timezone.name   : CDT
timezone.offset : -18000
hostname    : ubuntu1804
fs.count    : 1
fs.0.name   : vda2
fs.0.mountpoint : /
fs.0.fstype : ext4
fs.0.disk.count : 1
fs.0.disk.0.alias   : vda


Looks good to me.



Thanks,


DHB


On 8/23/19 1:31 PM, Jonathon Jongsma wrote:

changes in v3:
- fixed test failure
- fixed syntax issues that I had missed since I forgot to install cppi on my
   new laptop

This series adds several bits of guest information provided by a new API
function virDomainGetGuestInfo(). There is an implementation for qemu using the
guest agent. In particular, it adds information about logged-in users, guest
OS, timezone, hostname, and filesystem info.

Filesystem information is already accessible via the public API
virDomainGetFSInfo(), but recent versions of the qemu guest agent added
additional information, and the existing public API is not able to be extended
without breaking API since it returns a C struct. This new API uses typed
parameters so that it can be extended as necessary when new fields are added.

The overall API follows the bulk stats API quite closely, and tries to return
data in similar ways (for example, the "users.N.*" field name scheme was
inspired by various stats fields).

It should be noted that like version 1 of this patch series, the API still only
operates on a single domain, not a list of domains. I'm willing to consider
changing the API to operate on a list of domains if that is the concensus, but
I lean toward keeping it simpler.

Here's an example of the output of the virsh command added in this patch
operating on a fedora 30 guest:
virsh # guestinfo fedora30
user.count  : 1
user.0.name : test
user.0.login-time   : 1566422940895
os.id   : fedora
os.name : Fedora
os.pretty-name  : Fedora 30 (Workstation Edition)
os.version  : 30 (Workstation Edition)
os.version-id   : 30
os.machine  : x86_64
os.variant  : Workstation Edition
os.variant-id   : workstation
os.kernel-release   : 5.2.7-200.fc30.x86_64
os.kernel-version   : #1 SMP Thu Aug 8 05:35:29 UTC 2019
timezone.name   : CDT
timezone.offset : -18000
hostname: myhostname
fs.count: 3
fs.0.name   : dm-0
fs.0.mountpoint : /
fs.0.fstype : ext4
fs.0.total-bytes: 25928306688
fs.0.used-bytes : 10762133504
fs.0.disk.count : 1
fs.0.disk.0.alias   : vda
fs.0.disk.0.serial  : 947684ABZ8639Q
fs.0.disk.0.device  : /dev/vda2
fs.1.name   : vda1
fs.1.mountpoint : /boot
fs.1.fstype : ext4
fs.1.total-bytes: 952840192
fs.1.used-bytes : 229019648
fs.1.disk.count : 1
fs.1.disk.0.alias   : vda
fs.1.disk.0.serial  : 947684ABZ8639Q
fs.1.disk.0.device  : /dev/vda1
fs.2.name   : md127
fs.2.mountpoint : /run/media/test/971b1edc-da61-4015-b465-560a9b1b6e9b
fs.2.fstype : ext4
fs.2.total-bytes: 1950134272
fs.2.used-bytes : 6291456
fs.2.disk.count : 2
fs.2.disk.0.alias   : vdb
fs.2.disk.0.device  : /dev/vdb1
fs.2.disk.1.alias   : vdc
fs.2.disk.1.device  : /dev/vdc1

In contrast, here is the output of the virsh command operating on a fedora24
guest:
virsh # guestinfo  fedora24
error: Reconnected to the hypervisor
fs.count: 2
fs.0.name   : dm-0
fs.0.mountpoint : /
fs.0.fstype : ext4
fs.0.disk.count : 1
fs.0.disk.0.alias   : vda
fs.1.name   : vda1
fs.1.mountpoint : /boot
fs.1.fstype : ext4
fs.1.disk.count : 1
fs.1.disk.0.alias   : vda

However, if you specifically request an info category that is not supported by
the guest agent:
virsh # guestinfo --user fedora24
error: internal error: unable to execute QEMU agent command 'guest-get-users': 
The command guest-get-users has not been found



Jonathon Jongsma (9):
   lib: add virDomainGetGuestInfo()
   remote: implement virDomainGetGuestInfo
   qemu: add helper for getting guest users
   qemu: add helper function for querying OS info
   qemu: add helper for querying timezone info
   qemu: add support for new fields in FSInfo
   qemu: add helper for getting full FSInfo
   qemu: Implement virDomainGetGuestInfo()
   virsh: add 'guestinfo' command

  include/libvirt/libvirt-domain.h|  14 +
  src/driver-hypervisor.h |   8 +
  src/libvirt-domain.c| 117 ++
  src/libvirt_public.syms |   1 +
  src/qemu/qemu_agent.c 

Re: [libvirt] [PATCH v3 9/9] virsh: add 'guestinfo' command

[Daniel Henrique Barboza]

On 8/23/19 1:31 PM, Jonathon Jongsma wrote:

The 'guestinfo' command uses the new virDomainGetGuestInfo() API to
query information about the specified domain and print it out for the
user. The output is modeled roughly on the 'domstats' command.

Signed-off-by: Jonathon Jongsma 
---


Reviewed-by: Daniel Henrique Barboza 
Tested-by: Daniel Henrique Barboza 



  tools/virsh-domain.c | 85 
  1 file changed, 85 insertions(+)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index ccda71d7e0..977783951d 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -14038,6 +14038,85 @@ cmdDomFSInfo(vshControl *ctl, const vshCmd *cmd)
  return ret;
  }
  
+/*

+ * "guestinfo" command
+ */
+static const vshCmdInfo info_guestinfo[] = {
+{.name = "help",
+ .data = N_("query information about the guest (via agent)")
+},
+{.name = "desc",
+ .data = N_("Use the guest agent to query various information from guest's 
"
+"point of view")
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_guestinfo[] = {
+VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
+{.name = "user",
+ .type = VSH_OT_BOOL,
+ .help = N_("report active users"),
+},
+{.name = "os",
+ .type = VSH_OT_BOOL,
+ .help = N_("report operating system information"),
+},
+{.name = "timezone",
+ .type = VSH_OT_BOOL,
+ .help = N_("report timezone information"),
+},
+{.name = "hostname",
+ .type = VSH_OT_BOOL,
+ .help = N_("report hostname"),
+},
+{.name = "filesystem",
+ .type = VSH_OT_BOOL,
+ .help = N_("report filesystem information"),
+},
+{.name = NULL}
+};
+
+static bool
+cmdGuestInfo(vshControl *ctl, const vshCmd *cmd)
+{
+virDomainPtr dom;
+bool ret = false;
+virTypedParameterPtr params = NULL;
+int nparams = 0;
+size_t i;
+unsigned int types = 0;
+
+if (vshCommandOptBool(cmd, "user"))
+types |= VIR_DOMAIN_GUEST_INFO_USERS;
+if (vshCommandOptBool(cmd, "os"))
+types |= VIR_DOMAIN_GUEST_INFO_OS;
+if (vshCommandOptBool(cmd, "timezone"))
+types |= VIR_DOMAIN_GUEST_INFO_TIMEZONE;
+if (vshCommandOptBool(cmd, "hostname"))
+types |= VIR_DOMAIN_GUEST_INFO_HOSTNAME;
+if (vshCommandOptBool(cmd, "filesystem"))
+types |= VIR_DOMAIN_GUEST_INFO_FILESYSTEM;
+
+if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+return false;
+
+if (virDomainGetGuestInfo(dom, types, , , 0) < 0)
+goto cleanup;
+
+for (i = 0; i < nparams; i++) {
+char *str = vshGetTypedParamValue(ctl, [i]);
+vshPrint(ctl, "%-20s: %s\n", params[i].field, str);
+VIR_FREE(str);
+}
+
+ret = true;
+
+ cleanup:
+virshDomainFree(dom);
+return ret;
+}
+
  const vshCmdDef domManagementCmds[] = {
  {.name = "attach-device",
   .handler = cmdAttachDevice,
@@ -14653,5 +14732,11 @@ const vshCmdDef domManagementCmds[] = {
   .info = info_domblkthreshold,
   .flags = 0
  },
+{.name = "guestinfo",
+ .handler = cmdGuestInfo,
+ .opts = opts_guestinfo,
+ .info = info_guestinfo,
+ .flags = 0
+},
  {.name = NULL}
  };


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

Re: [libvirt] [PATCH v3 8/9] qemu: Implement virDomainGetGuestInfo()

[Daniel Henrique Barboza]

On 8/23/19 1:31 PM, Jonathon Jongsma wrote:

Iimplements the new guest information API by querying requested
information via the guest agent.

Signed-off-by: Jonathon Jongsma 
---


Reviewed-by: Daniel Henrique Barboza 
Tested-by: Daniel Henrique Barboza 


  src/qemu/qemu_driver.c | 110 +
  1 file changed, 110 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1b051a9424..446266e66b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -23190,6 +23190,115 @@ qemuDomainGetLaunchSecurityInfo(virDomainPtr domain,
  return ret;
  }
  
+static unsigned int supportedGuestInfoTypes =

+VIR_DOMAIN_GUEST_INFO_USERS |
+VIR_DOMAIN_GUEST_INFO_OS |
+VIR_DOMAIN_GUEST_INFO_TIMEZONE |
+VIR_DOMAIN_GUEST_INFO_HOSTNAME |
+VIR_DOMAIN_GUEST_INFO_FILESYSTEM;
+
+static void
+qemuDomainGetGuestInfoCheckSupport(unsigned int *types)
+{
+if (*types == 0)
+*types = supportedGuestInfoTypes;
+
+*types = *types & supportedGuestInfoTypes;
+}
+
+static int
+qemuDomainGetGuestInfo(virDomainPtr dom,
+   unsigned int types,
+   virTypedParameterPtr *params,
+   int *nparams,
+   unsigned int flags)
+{
+virQEMUDriverPtr driver = dom->conn->privateData;
+virDomainObjPtr vm = NULL;
+qemuAgentPtr agent;
+int ret = -1;
+int rv = -1;
+int maxparams = 0;
+char *hostname = NULL;
+virDomainDefPtr def = NULL;
+virCapsPtr caps = NULL;
+unsigned int supportedTypes = types;
+
+virCheckFlags(0, ret);
+qemuDomainGetGuestInfoCheckSupport();
+
+if (!(vm = qemuDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainGetGuestInfoEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_QUERY) < 0)
+goto cleanup;
+
+if (!qemuDomainAgentAvailable(vm, true))
+goto endjob;
+
+agent = qemuDomainObjEnterAgent(vm);
+
+/* Although the libvirt qemu driver supports all of these guest info types,
+ * some guest agents might be too old to support these commands. If these
+ * info categories were explicitly requested (i.e. 'types' is non-zero),
+ * abort and report an error on any failures, otherwise continue and return
+ * as much info as is supported by the guest agent. */
+if (supportedTypes & VIR_DOMAIN_GUEST_INFO_USERS) {
+if (qemuAgentGetUsers(agent, params, nparams, ) < 0 &&
+types != 0)
+goto exitagent;
+}
+if (supportedTypes & VIR_DOMAIN_GUEST_INFO_OS) {
+if (qemuAgentGetOSInfo(agent, params, nparams, ) < 0
+&& types != 0)
+goto exitagent;
+}
+if (supportedTypes & VIR_DOMAIN_GUEST_INFO_TIMEZONE) {
+if (qemuAgentGetTimezone(agent, params, nparams, ) < 0
+&& types != 0)
+goto exitagent;
+}
+if (supportedTypes & VIR_DOMAIN_GUEST_INFO_HOSTNAME) {
+if (qemuAgentGetHostname(agent, ) < 0) {
+if (types != 0)
+goto exitagent;
+} else {
+if (virTypedParamsAddString(params, nparams, , 
"hostname",
+hostname) < 0)
+goto exitagent;
+}
+}
+if (supportedTypes & VIR_DOMAIN_GUEST_INFO_FILESYSTEM) {
+if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+goto exitagent;
+
+if (!(def = virDomainDefCopy(vm->def, caps, driver->xmlopt, NULL, 
false)))
+goto exitagent;
+
+if (qemuAgentGetFSInfoParams(agent, params, nparams, , def) < 0 
&&
+types != 0)
+goto exitagent;
+}
+
+rv = 0;
+
+ exitagent:
+qemuDomainObjExitAgent(vm, agent);
+
+ endjob:
+qemuDomainObjEndAgentJob(vm);
+
+ cleanup:
+virDomainObjEndAPI();
+virDomainDefFree(def);
+virObjectUnref(caps);
+VIR_FREE(hostname);
+return rv;
+}
+
  static virHypervisorDriver qemuHypervisorDriver = {
  .name = QEMU_DRIVER_NAME,
  .connectURIProbe = qemuConnectURIProbe,
@@ -23425,6 +23534,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
  .domainCheckpointLookupByName = qemuDomainCheckpointLookupByName, /* 
5.6.0 */
  .domainCheckpointGetParent = qemuDomainCheckpointGetParent, /* 5.6.0 */
  .domainCheckpointDelete = qemuDomainCheckpointDelete, /* 5.6.0 */
+.domainGetGuestInfo = qemuDomainGetGuestInfo, /* 5.6.0 */
  };
  
  


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

Re: [libvirt] [PATCH v3 7/9] qemu: add helper for getting full FSInfo

[Daniel Henrique Barboza]

On 8/23/19 1:31 PM, Jonathon Jongsma wrote:

This function adds the complete filesystem information returned by the
qemu agent to an array of typed parameters with field names intended to
to be returned by virDomainGetGuestInfo()

Signed-off-by: Jonathon Jongsma 
---


Tested-by: Daniel Henrique Barboza 


A few comments below:



  src/qemu/qemu_agent.c |  90 ++
  src/qemu/qemu_agent.h |   5 +
  tests/qemuagenttest.c | 210 +++---
  3 files changed, 292 insertions(+), 13 deletions(-)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 8d54979f89..169ad74f6f 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1953,6 +1953,96 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr 
**info,
  return ret;
  }
  
+int

+qemuAgentGetFSInfoParams(qemuAgentPtr mon,
+ virTypedParameterPtr *params,
+ int *nparams, int *maxparams,
+ virDomainDefPtr vmdef)
+{
+int ret = -1;
+qemuAgentFSInfoPtr *fsinfo = NULL;
+size_t i, j;
+int nfs;
+
+if ((nfs = qemuAgentGetFSInfoInternal(mon, , vmdef)) < 0)
+return -1;
+
+if (virTypedParamsAddUInt(params, nparams, maxparams,
+  "fs.count", nfs) < 0)
+goto cleanup;
+
+for (i = 0; i < nfs; i++) {
+char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.name", i);
+if (virTypedParamsAddString(params, nparams, maxparams,
+param_name, fsinfo[i]->name) < 0)
+goto cleanup;
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.mountpoint", i);
+if (virTypedParamsAddString(params, nparams, maxparams,
+param_name, fsinfo[i]->mountpoint) < 0)
+goto cleanup;
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.fstype", i);
+if (virTypedParamsAddString(params, nparams, maxparams,
+param_name, fsinfo[i]->fstype) < 0)
+goto cleanup;
+
+/* disk usage values are not returned by older guest agents, so
+ * only add the params if the value is set */
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.total-bytes", i);
+if (fsinfo[i]->total_bytes != -1 &&
+virTypedParamsAddULLong(params, nparams, maxparams,
+param_name, fsinfo[i]->total_bytes) < 0)
+goto cleanup;
+
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.used-bytes", i);
+if (fsinfo[i]->used_bytes != -1 &&
+virTypedParamsAddULLong(params, nparams, maxparams,
+param_name, fsinfo[i]->used_bytes) < 0)
+goto cleanup;
+
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.disk.count", i);
+if (virTypedParamsAddUInt(params, nparams, maxparams,
+  param_name, fsinfo[i]->ndisks) < 0)
+goto cleanup;
+for (j = 0; j < fsinfo[i]->ndisks; j++) {
+qemuAgentDiskInfoPtr d = fsinfo[i]->disks[j];
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.disk.%zu.alias", i, j);
+if (d->alias &&
+virTypedParamsAddString(params, nparams, maxparams,
+param_name, d->alias) < 0)
+goto cleanup;
+
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.disk.%zu.serial", i, j);
+if (d->serial &&
+virTypedParamsAddString(params, nparams, maxparams,
+param_name, d->serial) < 0)
+goto cleanup;
+
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.disk.%zu.device", i, j);
+if (d->devnode &&
+virTypedParamsAddString(params, nparams, maxparams,
+param_name, d->devnode) < 0)
+goto cleanup;
+}
+}
+ret = nfs;
+
+ cleanup:
+for (i = 0; i < nfs; i++)
+qemuAgentFSInfoFree(fsinfo[i]);
+VIR_FREE(fsinfo);
+
+return ret;
+}
  
  static int

  qemuAgentGetFSInfoInternal(qemuAgentPtr mon, qemuAgentFSInfoPtr **info,
diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
index 69b0176855..f6d74a2603 100644
--- a/src/qemu/qemu_agent.h
+++ b/src/qemu/qemu_agent.h
@@ -74,6 +74,11 @@ int qemuAgentFSThaw(qemuAgentPtr mon);
  int qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info,
 virDomainDefPtr vmdef);
  
+int qemuAgentGetFSInfoParams(qemuAgentPtr mon,

+ 

Re: [libvirt] [PATCH v3 6/9] qemu: add support for new fields in FSInfo

[Daniel Henrique Barboza]

On 8/23/19 1:31 PM, Jonathon Jongsma wrote:

Since version 3.0, qemu has returned disk usage statistics in
guest-get-fsinfo. And since 3.1, it has returned information about the
disk serial number and device node of disks that are targeted by the
filesystem.

Unfortunately, the public API virDomainGetFSInfo() returns the
filesystem info using a virDomainFSInfo struct, and due to API/ABI
guaranteeds it cannot be extended. So this new information cannot


s/guaranteeds/guarantees


easily be added to the public API. However, it is possible to add this
new filesystem information to a new virDomainGetGuestInfo() API which
will be based on typed parameters and is thus more extensible.

In order to support these two use cases, I added an internal struct
which the agent code uses to return all of the new data fields. This
internal struct can be converted to the public struct at a cost of some
extra memory allocation.

In a following commit, this additional information will be used within
virDomainGetGuestInfo().

Signed-off-by: Jonathon Jongsma 
---


Tested-by: Daniel Henrique Barboza 


Got a few style questions below. Assuming it's ok or the commiter
can fix those before pushing,


Reviewed-by: Daniel Henrique Barboza 



  src/qemu/qemu_agent.c | 203 +-
  1 file changed, 182 insertions(+), 21 deletions(-)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index e986204162..8d54979f89 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1827,19 +1827,144 @@ qemuAgentSetTime(qemuAgentPtr mon,
  return ret;
  }
  
+typedef struct _qemuAgentDiskInfo qemuAgentDiskInfo;

+typedef qemuAgentDiskInfo *qemuAgentDiskInfoPtr;
+struct _qemuAgentDiskInfo {
+char *alias;
+char *serial;
+char *devnode;
+};
+
+typedef struct _qemuAgentFSInfo qemuAgentFSInfo;
+typedef qemuAgentFSInfo *qemuAgentFSInfoPtr;
+struct _qemuAgentFSInfo {
+char *mountpoint; /* path to mount point */
+char *name;   /* device name in the guest (e.g. "sda1") */
+char *fstype; /* filesystem type */
+long long total_bytes;
+long long used_bytes;
+size_t ndisks;
+qemuAgentDiskInfoPtr *disks;
+};
+
+static void
+qemuAgentDiskInfoFree(qemuAgentDiskInfoPtr info)
+{
+if (!info)
+return;
+
+VIR_FREE(info->serial);
+VIR_FREE(info->alias);
+VIR_FREE(info->devnode);
+VIR_FREE(info);
+}
+
+static void
+qemuAgentFSInfoFree(qemuAgentFSInfoPtr info)
+{
+size_t i;
+
+if (!info)
+return;
+
+VIR_FREE(info->mountpoint);
+VIR_FREE(info->name);
+VIR_FREE(info->fstype);
+
+for (i = 0; i < info->ndisks; i++)
+qemuAgentDiskInfoFree(info->disks[i]);
+VIR_FREE(info->disks);
+
+VIR_FREE(info);
+}
+
+static virDomainFSInfoPtr
+qemuAgentFSInfoToPublic(qemuAgentFSInfoPtr agent)
+{
+virDomainFSInfoPtr ret = NULL;
+size_t n;
+
+if (VIR_ALLOC(ret) < 0)
+return NULL;
  
+if (VIR_STRDUP(ret->name, agent->name) < 0)

+goto error;
+if (VIR_STRDUP(ret->mountpoint, agent->mountpoint) < 0)
+goto error;
+if (VIR_STRDUP(ret->fstype, agent->fstype) < 0)
+goto error;
+
+ret->ndevAlias = agent->ndisks;
+
+if (ret->ndevAlias == 0)
+return ret;
+
+if (VIR_ALLOC_N(ret->devAlias, ret->ndevAlias) < 0)
+goto error;
+
+for (n = 0; n < ret->ndevAlias; n++) {
+if (VIR_STRDUP(ret->devAlias[n], agent->disks[n]->alias) < 0)
+goto error;
+}
+
+return ret;
+
+ error:
+virDomainFSInfoFree(ret);
+return NULL;
+}
+
+static int
+qemuAgentGetFSInfoInternal(qemuAgentPtr mon, qemuAgentFSInfoPtr **info,
+   virDomainDefPtr vmdef);
  int
  qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info,
 virDomainDefPtr vmdef)
+{
+int ret = -1;
+qemuAgentFSInfoPtr *agentinfo = NULL;
+virDomainFSInfoPtr *info_ret = NULL;
+size_t i;
+int nfs;
+
+nfs = qemuAgentGetFSInfoInternal(mon, , vmdef);
+if (nfs < 0)
+return ret;
+if (VIR_ALLOC_N(info_ret, nfs) < 0)
+goto cleanup;
+
+for (i = 0; i < nfs; i++) {
+if (!(info_ret[i] = qemuAgentFSInfoToPublic(agentinfo[i])))
+goto cleanup;
+}
+
+*info = info_ret;
+info_ret = NULL;
+ret = nfs;
+
+ cleanup:
+for (i = 0; i < nfs; i++) {
+qemuAgentFSInfoFree(agentinfo[i]);
+/* if there was an error, free any memory we've allocated for the
+ * return value */
+if (info_ret)
+virDomainFSInfoFree(info_ret[i]);
+}
+return ret;
+}
+
+
+static int
+qemuAgentGetFSInfoInternal(qemuAgentPtr mon, qemuAgentFSInfoPtr **info,
+   virDomainDefPtr vmdef)
  {
  size_t i, j, k;
  int ret = -1;
-size_t ndata = 0, ndisk;
-char **alias;
+size_t ndata = 0;
  virJSONValuePtr cmd;
  virJSONValuePtr reply = NULL;
  virJSONValuePtr data;
-virDomainFSInfoPtr 

Re: [libvirt] [PATCH v3 5/9] qemu: add helper for querying timezone info

[Daniel Henrique Barboza]

On 8/23/19 1:31 PM, Jonathon Jongsma wrote:

This function queries timezone information within the guest and adds
the information to an array of typed parameters with field names
intended to be returned to virDomainGetGuestInfo()

Signed-off-by: Jonathon Jongsma 
---


I spotted two missing indentations down there, but other than that,
LGTM.


Reviewed-by: Daniel Henrique Barboza 
Tested-by: Daniel Henrique Barboza 



  src/qemu/qemu_agent.c | 46 ++
  src/qemu/qemu_agent.h |  1 +
  tests/qemuagenttest.c | 76 +++
  3 files changed, 123 insertions(+)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 0fcc3fdabd..e986204162 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -2384,3 +2384,49 @@ qemuAgentGetOSInfo(qemuAgentPtr mon,
  virJSONValueFree(reply);
  return ret;
  }
+
+int
+qemuAgentGetTimezone(qemuAgentPtr mon,
+ virTypedParameterPtr *params,
+ int *nparams,
+ int *maxparams)
+{
+int ret = -1;
+virJSONValuePtr cmd;
+virJSONValuePtr reply = NULL;
+virJSONValuePtr data = NULL;
+const char *name;
+int offset;
+
+if (!(cmd = qemuAgentMakeCommand("guest-get-timezone", NULL)))
+return -1;
+
+if (qemuAgentCommand(mon, cmd, , true,
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
+goto cleanup;
+
+if (!(data = virJSONValueObjectGetObject(reply, "return"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("guest-get-timezone reply was missing return data"));
+goto cleanup;
+}
+
+if ((name = virJSONValueObjectGetString(data, "zone")) == NULL)
+goto cleanup;
+if (virTypedParamsAddString(params, nparams, maxparams,
+"timezone.name", name) < 0)
+goto cleanup;
+
+if ((virJSONValueObjectGetNumberInt(data, "offset", )) < 0)
+goto cleanup;
+if (virTypedParamsAddInt(params, nparams, maxparams,
+ "timezone.offset", offset) < 0)
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
index ee019455e5..69b0176855 100644
--- a/src/qemu/qemu_agent.h
+++ b/src/qemu/qemu_agent.h
@@ -123,3 +123,4 @@ int qemuAgentSetUserPassword(qemuAgentPtr mon,
  
  int qemuAgentGetUsers(qemuAgentPtr mon, virTypedParameterPtr *params, int *nparams, int *maxparams);

  int qemuAgentGetOSInfo(qemuAgentPtr mon, virTypedParameterPtr *params, int 
*nparams, int *maxparams);
+int qemuAgentGetTimezone(qemuAgentPtr mon, virTypedParameterPtr *params, int 
*nparams, int *maxparams);
diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
index 489b77d403..0912a5f29a 100644
--- a/tests/qemuagenttest.c
+++ b/tests/qemuagenttest.c
@@ -1189,7 +1189,82 @@ testQemuAgentOSInfo(const void *data)
  return ret;
  }
  
+static const char testQemuAgentTimezoneResponse1[] =

+"{\"return\":{\"zone\":\"IST\",\"offset\":19800}}";
+static const char testQemuAgentTimezoneResponse2[] =
+"{\"return\":{\"zone\":\"CEST\",\"offset\":7200}}";
+static const char testQemuAgentTimezoneResponse3[] =
+"{\"return\":{\"zone\":\"NDT\",\"offset\":-9000}}";
+static const char testQemuAgentTimezoneResponse4[] =
+"{\"return\":{\"zone\":\"PDT\",\"offset\":-25200}}";
  
+static int

+testQemuAgentTimezone(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt);
+int ret = -1;
+
+if (!test)
+return -1;
+
+#define VALIDATE_TIMEZONE(response_, expected_name_, expected_offset_) \
+do { \
+virTypedParameterPtr params_ = NULL; \
+int nparams_ = 0; \
+int maxparams_ = 0; \
+const char *name_ = NULL; \
+int offset_; \
+if (qemuMonitorTestAddAgentSyncResponse(test) < 0) \
+goto cleanup; \


"goto cleanup" should be indented 4 spaces after the preceeding 'if'


+if (qemuMonitorTestAddItem(test, "guest-get-timezone", \
+   response_) < 0) \
+goto cleanup; \


"goto cleanup" should be indented 4 spaces after the preceeding 'if'


+if (qemuAgentGetTimezone(qemuMonitorTestGetAgent(test), \
+ _, _, _) < 0) \
+goto cleanup; \
+if (nparams_ != 2) { \
+virReportError(VIR_ERR_INTERNAL_ERROR, \
+   "Expected 2 params, got %d", nparams_); \
+goto cleanup; \
+} \
+if (virTypedParamsGetString(params_, nparams_, \
+"timezone.name", _) < 0) { \
+virReportError(VIR_ERR_INTERNAL_ERROR, "missing param '%s'", \
+   "tiemzone.name"); \
+goto cleanup; \
+} \
+if 

[libvirt] [PATCH] xenconfig: move contents to libxl driver and remove directory

[Jim Fehlig]

Signed-off-by: Jim Fehlig 
---
 cfg.mk   |  2 +-
 configure.ac |  2 --
 po/POTFILES  |  6 ++---
 src/Makefile.am  |  1 -
 src/libvirt_xenconfig.syms   | 12 --
 src/libxl/Makefile.inc.am| 25 ++---
 src/{xenconfig => libxl}/xen_common.c|  0
 src/{xenconfig => libxl}/xen_common.h|  0
 src/{xenconfig => libxl}/xen_xl.c|  0
 src/{xenconfig => libxl}/xen_xl.h|  0
 src/{xenconfig => libxl}/xen_xm.c|  0
 src/{xenconfig => libxl}/xen_xm.h|  0
 src/{xenconfig => libxl}/xenxs_private.h |  0
 src/xenconfig/Makefile.inc.am| 28 
 tests/xlconfigtest.c |  2 +-
 tests/xmconfigtest.c |  2 +-
 16 files changed, 13 insertions(+), 67 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index c459ad443f..1f29729949 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -791,7 +791,7 @@ sc_prohibit_cross_inclusion:
access/ | conf/) safe="($$dir|conf|util)";; \
cpu/| network/| node_device/| rpc/| security/| storage/) \
  safe="($$dir|util|conf|storage)";; \
-   xenapi/ | xenconfig/ ) safe="($$dir|util|conf|xen|cpu)";; \
+   xenapi/) safe="($$dir|util|conf|xen|cpu)";; \
*) safe="($$dir|$(mid_dirs)|util)";; \
  esac; \
  in_vc_files="^src/$$dir" \
diff --git a/configure.ac b/configure.ac
index a60543072d..890702a89d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -465,8 +465,6 @@ LIBVIRT_DRIVER_CHECK_LIBVIRTD
 LIBVIRT_DRIVER_CHECK_NETWORK
 LIBVIRT_DRIVER_CHECK_INTERFACE
 
-AM_CONDITIONAL([WITH_XENCONFIG], [test "$with_libxl" = "yes"])
-
 
 dnl
 dnl in case someone want to build static binaries
diff --git a/po/POTFILES b/po/POTFILES
index c62bc32bb2..e466e1bc55 100644
--- a/po/POTFILES
+++ b/po/POTFILES
@@ -88,6 +88,9 @@ src/libxl/libxl_conf.c
 src/libxl/libxl_domain.c
 src/libxl/libxl_driver.c
 src/libxl/libxl_migration.c
+src/libxl/xen_common.c
+src/libxl/xen_xl.c
+src/libxl/xen_xm.c
 src/locking/lock_daemon.c
 src/locking/lock_daemon_dispatch.c
 src/locking/lock_driver_lockd.c
@@ -291,9 +294,6 @@ src/vz/vz_utils.c
 src/vz/vz_utils.h
 src/xenapi/xenapi_driver.c
 src/xenapi/xenapi_utils.c
-src/xenconfig/xen_common.c
-src/xenconfig/xen_xl.c
-src/xenconfig/xen_xm.c
 tests/virpolkittest.c
 tools/libvirt-guests.sh.in
 tools/virsh-checkpoint.c
diff --git a/src/Makefile.am b/src/Makefile.am
index adaf61350a..6626659113 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -118,7 +118,6 @@ include vbox/Makefile.inc.am
 include openvz/Makefile.inc.am
 include qemu/Makefile.inc.am
 include bhyve/Makefile.inc.am
-include xenconfig/Makefile.inc.am
 include libxl/Makefile.inc.am
 include xenapi/Makefile.inc.am
 include vz/Makefile.inc.am
diff --git a/src/libvirt_xenconfig.syms b/src/libvirt_xenconfig.syms
deleted file mode 100644
index 6e2e578b15..00
--- a/src/libvirt_xenconfig.syms
+++ /dev/null
@@ -1,12 +0,0 @@
-#
-# These symbols are dependent upon --with-xen via WITH_XEN or --with-libxl via 
WITH_LIBXL.
-#
-
-# xenconfig/xen_xm.h
-xenFormatXM;
-xenParseXM;
-
-# Let emacs know we want case-insensitive sorting
-# Local Variables:
-# sort-fold-case: t
-# End:
diff --git a/src/libxl/Makefile.inc.am b/src/libxl/Makefile.inc.am
index 1587404586..560b214877 100644
--- a/src/libxl/Makefile.inc.am
+++ b/src/libxl/Makefile.inc.am
@@ -1,6 +1,13 @@
 # vim: filetype=automake
 
 LIBXL_DRIVER_SOURCES = \
+   libxl/xenxs_private.h \
+   libxl/xen_common.c \
+   libxl/xen_common.h \
+   libxl/xen_xl.c \
+   libxl/xen_xl.h \
+   libxl/xen_xm.c \
+   libxl/xen_xm.h \
libxl/libxl_conf.c \
libxl/libxl_conf.h \
libxl/libxl_capabilities.c \
@@ -15,29 +22,13 @@ LIBXL_DRIVER_SOURCES = \
libxl/libxl_migration.h \
$(NULL)
 
-XENCONFIG_LIBXL_SOURCES = \
-   $(XENCONFIG_SOURCES) \
-   xenconfig/xen_xl.c \
-   xenconfig/xen_xl.h \
-   $(NULL)
-
 DRIVER_SOURCE_FILES += $(LIBXL_DRIVER_SOURCES)
 STATEFUL_DRIVER_SOURCE_FILES += $(LIBXL_DRIVER_SOURCES)
 EXTRA_DIST += \
$(LIBXL_DRIVER_SOURCES) \
-   $(XENCONFIG_LIBXL_SOURCES) \
$(NULL)
 
 if WITH_LIBXL
-noinst_LTLIBRARIES += libvirt_xenconfig_libxl.la
-libvirt_xenconfig_libxl_la_LIBADD = $(LIBXL_LIBS)
-libvirt_xenconfig_libxl_la_CFLAGS = \
-   -I$(srcdir)/conf \
-   -I$(srcdir)/libxl \
-   $(AM_CFLAGS) \
-   $(NULL)
-libvirt_xenconfig_libxl_la_SOURCES = $(XENCONFIG_LIBXL_SOURCES)
-
 noinst_LTLIBRARIES += libvirt_driver_libxl_impl.la
 libvirt_driver_libxl_la_SOURCES =
 libvirt_driver_libxl_la_LIBADD = \
@@ -54,14 +45,12 @@ libvirt_driver_libxl_impl_la_CFLAGS = \
-I$(srcdir)/access \
-I$(srcdir)/conf \
-I$(srcdir)/secret \
-   -I$(srcdir)/xenconfig \
$(AM_CFLAGS) \
$(NULL)
 libvirt_driver_libxl_impl_la_LDFLAGS = $(AM_LDFLAGS)
 

Re: [libvirt] [PATCH v3 4/9] qemu: add helper function for querying OS info

[Daniel Henrique Barboza]

On 8/23/19 1:31 PM, Jonathon Jongsma wrote:

This function queries the guest operating system information and adds
the returned information to an array of typed parameters with field
names intended to be returned in virDomainGetGuestInfo().

Signed-off-by: Jonathon Jongsma 
---


Reviewed-by: Daniel Henrique Barboza 
Tested-by: Daniel Henrique Barboza 


  src/qemu/qemu_agent.c |  53 ++
  src/qemu/qemu_agent.h |   1 +
  tests/qemuagenttest.c | 122 ++
  3 files changed, 176 insertions(+)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 963a4b9359..0fcc3fdabd 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -2331,3 +2331,56 @@ qemuAgentGetUsers(qemuAgentPtr mon,
  virJSONValueFree(reply);
  return ret;
  }
+
+int
+qemuAgentGetOSInfo(qemuAgentPtr mon,
+   virTypedParameterPtr *params,
+   int *nparams,
+   int *maxparams)
+{
+int ret = -1;
+virJSONValuePtr cmd;
+virJSONValuePtr reply = NULL;
+virJSONValuePtr data = NULL;
+const char *result;
+
+if (!(cmd = qemuAgentMakeCommand("guest-get-osinfo", NULL)))
+return -1;
+
+if (qemuAgentCommand(mon, cmd, , true,
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
+goto cleanup;
+
+if (!(data = virJSONValueObjectGetObject(reply, "return"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("guest-get-osinfo reply was missing return data"));
+goto cleanup;
+}
+
+#define OSINFO_ADD_PARAM(agent_string_, param_string_) \
+do { \
+if ((result = virJSONValueObjectGetString(data, agent_string_))) { \
+if (virTypedParamsAddString(params, nparams, maxparams, \
+param_string_, result) < 0) { \
+goto cleanup; \
+} \
+} \
+} while (0)
+OSINFO_ADD_PARAM("id", "os.id");
+OSINFO_ADD_PARAM("name", "os.name");
+OSINFO_ADD_PARAM("pretty-name", "os.pretty-name");
+OSINFO_ADD_PARAM("version", "os.version");
+OSINFO_ADD_PARAM("version-id", "os.version-id");
+OSINFO_ADD_PARAM("machine", "os.machine");
+OSINFO_ADD_PARAM("variant", "os.variant");
+OSINFO_ADD_PARAM("variant-id", "os.variant-id");
+OSINFO_ADD_PARAM("kernel-release", "os.kernel-release");
+OSINFO_ADD_PARAM("kernel-version", "os.kernel-version");
+
+ret = 0;
+
+ cleanup:
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
index 05621b521a..ee019455e5 100644
--- a/src/qemu/qemu_agent.h
+++ b/src/qemu/qemu_agent.h
@@ -122,3 +122,4 @@ int qemuAgentSetUserPassword(qemuAgentPtr mon,
   bool crypted);
  
  int qemuAgentGetUsers(qemuAgentPtr mon, virTypedParameterPtr *params, int *nparams, int *maxparams);

+int qemuAgentGetOSInfo(qemuAgentPtr mon, virTypedParameterPtr *params, int 
*nparams, int *maxparams);
diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
index f2936a59f0..489b77d403 100644
--- a/tests/qemuagenttest.c
+++ b/tests/qemuagenttest.c
@@ -1068,6 +1068,127 @@ testQemuAgentUsers(const void *data)
  return ret;
  }
  
+static const char testQemuAgentOSInfoResponse[] =

+"{\"return\": "
+"   {\"name\":\"CentOS Linux\", "
+"   \"kernel-release\":\"3.10.0-862.14.4.el7.x86_64\", "
+"   \"version\":\"7 (Core)\", "
+"   \"pretty-name\":\"CentOS Linux 7 (Core)\", "
+"   \"version-id\":\"7\", "
+"   \"kernel-version\":\"#1 SMP Wed Sep 26 15:12:11 UTC 2018\", "
+"   \"machine\":\"x86_64\", "
+"   \"id\":\"centos\"} "
+"}";
+
+static const char testQemuAgentOSInfoResponse2[] =
+"{\"return\": "
+"   {\"name\":\"Microsoft Windows\", "
+"   \"kernel-release\":\"7601\", "
+"   \"version\":\"Microsoft Windows 77\", "
+"   \"variant\":\"client\", "
+"   \"pretty-name\":\"Windows 7 Professional\", "
+"   \"version-id\":\"\", "
+"   \"variant-id\":\"client\", "
+"   \"kernel-version\":\"6.1\", "
+"   \"machine\":\"x86_64\", "
+"   \"id\":\"mswindows\"} "
+"}";
+
+static int
+testQemuAgentOSInfo(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt);
+virTypedParameterPtr params = NULL;
+int nparams = 0;
+int maxparams = 0;
+int ret = -1;
+
+if (!test)
+return -1;
+
+if (qemuMonitorTestAddAgentSyncResponse(test) < 0)
+goto cleanup;
+
+if (qemuMonitorTestAddItem(test, "guest-get-osinfo",
+   testQemuAgentOSInfoResponse) < 0)
+goto cleanup;
+
+/* get osinfo */
+if (qemuAgentGetOSInfo(qemuMonitorTestGetAgent(test),
+   , , ) < 0)
+goto cleanup;
+
+if (nparams != 8) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+  

Re: [libvirt] [PATCH v3 3/9] qemu: add helper for getting guest users

[Daniel Henrique Barboza]

On 8/23/19 1:31 PM, Jonathon Jongsma wrote:

This function fetches the list of logged-in users from the qemu agent
and adds them to a list of typed parameters so that they can be used
internally in libvirt.

Also add some basic tests for the function.

Signed-off-by: Jonathon Jongsma 



Tested-by: Daniel Henrique Barboza 


And, assuming that what I mentioned in my comment below is a
matter of style rather than a syntax sin:


Reviewed-by: Daniel Henrique Barboza 



---
  src/qemu/qemu_agent.c |  91 +++
  src/qemu/qemu_agent.h |   2 +
  tests/qemuagenttest.c | 168 ++
  3 files changed, 261 insertions(+)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 361db299a5..963a4b9359 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -2240,3 +2240,94 @@ qemuAgentSetUserPassword(qemuAgentPtr mon,
  VIR_FREE(password64);
  return ret;
  }
+
+int
+qemuAgentGetUsers(qemuAgentPtr mon,
+  virTypedParameterPtr *params,
+  int *nparams,
+  int *maxparams)
+{
+int ret = -1;
+size_t i;
+virJSONValuePtr cmd;
+virJSONValuePtr reply = NULL;
+virJSONValuePtr data = NULL;
+size_t ndata;
+const char *strvalue;
+
+if (!(cmd = qemuAgentMakeCommand("guest-get-users", NULL)))
+return -1;
+
+if (qemuAgentCommand(mon, cmd, , true,
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
+goto cleanup;
+
+if (!(data = virJSONValueObjectGetArray(reply, "return"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("guest-get-users reply was missing return data"));
+goto cleanup;
+}
+
+if (!virJSONValueIsArray(data)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Malformed guest-get-users data array"));
+goto cleanup;
+}
+
+ndata = virJSONValueArraySize(data);
+
+if (virTypedParamsAddUInt(params, nparams, maxparams,
+  "user.count", ndata) < 0)
+goto cleanup;
+
+for (i = 0; i < ndata; i++) {
+char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
+virJSONValuePtr entry = virJSONValueArrayGet(data, i);
+
+if (!entry) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("array element missing in guest-get-users return "
+ "value"));
+goto cleanup;
+}
+
+if (!(strvalue = virJSONValueObjectGetString(entry, "user"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("'user' missing in reply of guest-get-users"));
+goto cleanup;
+}
+
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, "user.%zu.name", i);
+if (virTypedParamsAddString(params, nparams, maxparams,
+param_name, strvalue) < 0)
+goto cleanup;
+
+/* 'domain' is only present for windows guests */
+if ((strvalue = virJSONValueObjectGetString(entry, "domain"))) {
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "user.%zu.domain", i);
+if (virTypedParamsAddString(params, nparams, maxparams,
+param_name, strvalue) < 0)
+goto cleanup;
+}
+
+double logintime;


I'd rather declare 'logintime' at the start of the 'for' loop, together with
'param_name' and 'entry'. Since this patch passed 'make syntax-check' I
believe this is more a matter of "artistic freedom" than a rule, so I guess
it's ok.


+if (virJSONValueObjectGetNumberDouble(entry, "login-time", ) 
< 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("'login-time' missing in reply of 
guest-get-users"));
+goto cleanup;
+}
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "user.%zu.login-time", i);
+if (virTypedParamsAddULLong(params, nparams, maxparams,
+param_name, logintime * 1000) < 0)
+goto cleanup;
+}
+
+ret = ndata;
+
+ cleanup:
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
index 6ae9fe54da..05621b521a 100644
--- a/src/qemu/qemu_agent.h
+++ b/src/qemu/qemu_agent.h
@@ -120,3 +120,5 @@ int qemuAgentSetUserPassword(qemuAgentPtr mon,
   const char *user,
   const char *password,
   bool crypted);
+
+int qemuAgentGetUsers(qemuAgentPtr mon, virTypedParameterPtr *params, int 
*nparams, int *maxparams);
diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
index 2f79986207..f2936a59f0 100644
--- a/tests/qemuagenttest.c
+++ b/tests/qemuagenttest.c
@@ -902,6 +902,173 @@ 

Re: [libvirt] [PATCH v3 2/9] remote: implement virDomainGetGuestInfo

[Daniel Henrique Barboza]

On 8/23/19 1:31 PM, Jonathon Jongsma wrote:

Add daemon and client code to serialize/deserialize
virDomainGetGuestInfo().

Signed-off-by: Jonathon Jongsma 
---


Reviewed-by: Daniel Henrique Barboza 
Tested-by: Daniel Henrique Barboza 



  src/remote/remote_daemon_dispatch.c | 41 ++
  src/remote/remote_driver.c  | 53 +
  src/remote/remote_protocol.x| 21 +++-
  src/remote_protocol-structs | 12 +++
  4 files changed, 126 insertions(+), 1 deletion(-)

diff --git a/src/remote/remote_daemon_dispatch.c 
b/src/remote/remote_daemon_dispatch.c
index 1bd281dd6d..665d938a99 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -7650,3 +7650,44 @@ remoteSerializeDomainDiskErrors(virDomainDiskErrorPtr 
errors,
  }
  return -1;
  }
+
+static int
+remoteDispatchDomainGetGuestInfo(virNetServerPtr server ATTRIBUTE_UNUSED,
+ virNetServerClientPtr client,
+ virNetMessagePtr msg ATTRIBUTE_UNUSED,
+ virNetMessageErrorPtr rerr,
+ remote_domain_get_guest_info_args *args,
+ remote_domain_get_guest_info_ret *ret)
+{
+int rv = -1;
+struct daemonClientPrivate *priv = 
virNetServerClientGetPrivateData(client);
+virDomainPtr dom = NULL;
+virTypedParameterPtr params = NULL;
+int nparams = 0;
+
+if (!priv->conn) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
+goto cleanup;
+}
+
+if (!(dom = get_nonnull_domain(priv->conn, args->dom)))
+goto cleanup;
+
+if (virDomainGetGuestInfo(dom, args->types, , , args->flags) 
< 0)
+goto cleanup;
+
+if (virTypedParamsSerialize(params, nparams,
+(virTypedParameterRemotePtr *) 
>params.params_val,
+>params.params_len,
+VIR_TYPED_PARAM_STRING_OKAY) < 0)
+goto cleanup;
+
+rv = 0;
+
+ cleanup:
+if (rv < 0)
+virNetMessageSaveError(rerr);
+virObjectUnref(dom);
+
+return rv;
+}
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index daac506672..5ba144648a 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -8306,6 +8306,58 @@ remoteNetworkPortGetParameters(virNetworkPortPtr port,
  return rv;
  }
  
+static int

+remoteDomainGetGuestInfo(virDomainPtr dom,
+ unsigned int types,
+ virTypedParameterPtr *params,
+ int *nparams,
+ unsigned int flags)
+{
+int rv = -1;
+struct private_data *priv = dom->conn->privateData;
+remote_domain_get_guest_info_args args;
+remote_domain_get_guest_info_ret ret;
+
+remoteDriverLock(priv);
+
+make_nonnull_domain(, dom);
+
+args.types = types;
+args.flags = flags;
+
+memset(, 0, sizeof(ret));
+
+if (call(dom->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_GUEST_INFO,
+ (xdrproc_t)xdr_remote_domain_get_guest_info_args, (char *),
+ (xdrproc_t)xdr_remote_domain_get_guest_info_ret, (char *)) == 
-1)
+goto done;
+
+if (ret.params.params_len > REMOTE_DOMAIN_GUEST_INFO_PARAMS_MAX) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Too many params in guestinfo: %d for limit %d"),
+   ret.params.params_len, 
REMOTE_DOMAIN_GUEST_INFO_PARAMS_MAX);
+goto cleanup;
+}
+
+if (params) {
+if (virTypedParamsDeserialize((virTypedParameterRemotePtr) 
ret.params.params_val,
+  ret.params.params_len,
+  REMOTE_DOMAIN_GUEST_INFO_PARAMS_MAX,
+  params,
+  nparams) < 0)
+goto cleanup;
+}
+
+rv = 0;
+
+ cleanup:
+xdr_free((xdrproc_t)xdr_remote_domain_get_guest_info_ret,
+ (char *) );
+
+ done:
+remoteDriverUnlock(priv);
+return rv;
+}
  
  /* get_nonnull_domain and get_nonnull_network turn an on-wire

   * (name, uuid) pair into virDomainPtr or virNetworkPtr object.
@@ -8733,6 +8785,7 @@ static virHypervisorDriver hypervisor_driver = {
  .domainCheckpointLookupByName = remoteDomainCheckpointLookupByName, /* 
5.6.0 */
  .domainCheckpointGetParent = remoteDomainCheckpointGetParent, /* 5.6.0 */
  .domainCheckpointDelete = remoteDomainCheckpointDelete, /* 5.6.0 */
+.domainGetGuestInfo = remoteDomainGetGuestInfo, /* 5.6.0 */
  };
  
  static virNetworkDriver network_driver = {

diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 118369e2b3..75c2bc69ff 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -269,6 +269,9 @@ const REMOTE_NODE_SEV_INFO_MAX = 64;
  /* Upper 

Re: [libvirt] [PATCH v3 1/9] lib: add virDomainGetGuestInfo()

[Daniel Henrique Barboza]

Flagged a couple of typos that can be fixed by the maintainer when
pushing upstream or by in a later version, if a new version is needed.


Aside for the typos, LGTM:

Reviewed-by: Daniel Henrique Barboza 
Tested-by: Daniel Henrique Barboza 




On 8/23/19 1:31 PM, Jonathon Jongsma wrote:

This API is intended to aggregate several guest agent information
queries and is ispired by stats API virDomainListGetStats(). It is


s/ispired/inspired


anticipated that this information will be provided by a guest agent
running within the domain.

Signed-off-by: Jonathon Jongsma 
---
  include/libvirt/libvirt-domain.h |  14 
  src/driver-hypervisor.h  |   8 +++
  src/libvirt-domain.c | 117 +++
  src/libvirt_public.syms  |   1 +
  4 files changed, 140 insertions(+)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index f160ee88b5..22277b0a84 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -4902,4 +4902,18 @@ int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
 int *nparams,
 unsigned int flags);
  
+typedef enum {

+VIR_DOMAIN_GUEST_INFO_USERS = (1 << 0), /* return active users */
+VIR_DOMAIN_GUEST_INFO_OS = (1 << 1), /* return OS information */
+VIR_DOMAIN_GUEST_INFO_TIMEZONE = (1 << 2), /* return timezone information 
*/
+VIR_DOMAIN_GUEST_INFO_HOSTNAME = (1 << 3), /* return hostname information 
*/
+VIR_DOMAIN_GUEST_INFO_FILESYSTEM = (1 << 4), /* return filesystem 
information */
+} virDomainGuestInfoTypes;
+
+int virDomainGetGuestInfo(virDomainPtr domain,
+  unsigned int types,
+  virTypedParameterPtr *params,
+  int *nparams,
+  unsigned int flags);
+
  #endif /* LIBVIRT_DOMAIN_H */
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index c1632ae4c6..58eb731e85 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -1359,6 +1359,13 @@ typedef int
  (*virDrvDomainCheckpointDelete)(virDomainCheckpointPtr checkpoint,
  unsigned int flags);
  
+typedef int

+(*virDrvDomainGetGuestInfo)(virDomainPtr domain,
+unsigned int types,
+virTypedParameterPtr *params,
+int *nparams,
+unsigned int flags);
+
  typedef struct _virHypervisorDriver virHypervisorDriver;
  typedef virHypervisorDriver *virHypervisorDriverPtr;
  
@@ -1617,4 +1624,5 @@ struct _virHypervisorDriver {

  virDrvDomainCheckpointLookupByName domainCheckpointLookupByName;
  virDrvDomainCheckpointGetParent domainCheckpointGetParent;
  virDrvDomainCheckpointDelete domainCheckpointDelete;
+virDrvDomainGetGuestInfo domainGetGuestInfo;
  };
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 2fe9bb8e91..ad68db7549 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -12212,6 +12212,123 @@ virDomainSetVcpu(virDomainPtr domain,
  return -1;
  }
  
+/**

+ * virDomainGetGuestInfo:
+ * @domain: pointer to domain object
+ * @types: types of information to return, binary-OR of virDomainGuestInfoTypes
+ * @params: location to store the guest info parameters
+ * @nparams: number of items in @params
+ * @flags: currently unused, set to 0
+ *
+ * Queries the guest agent for the various information about the guest system.
+ * The reported data depends on the guest agent implementation. the information


Capital "T" after the period. ". The information ..."


+ * is returned as an array of typed parameters containing the individual
+ * parameters. The parameter name for each information field consists of a
+ * dot-separated strign containing the name of the requested group followed by


s/strign/string



+ * a group-specific description of the statistic value.
+ *
+ * The information groups are enabled using the @types parameter which is a
+ * binary-OR of enum virDomainGuestInfoTypes. The following groups are 
available
+ * (although not necessarily implemented for each hypervisor):
+ *
+ * VIR_DOMAIN_GUEST_INFO_USERS:
+ *  returns information about users that are currently logged in within the
+ *  guest domain. The typed parameter keys are in this format:
+ *
+ *  "user.count" - the number of active users on this domain as an
+ * unsigned int
+ *  "user..name - username of the user as a string
+ *  "user..domain - domain of the user as a string (may only be
+ *   present on certain guest types)
+ *  "user..login-time - the login time of a user in milliseconds
+ *   since the epoch as unsigned long long
+ *
+ * VIR_DOMAIN_GUEST_INFO_OS:
+ *  Return information about the operating system running within the guest. The
+ *  typed parameter keys are in this 

Re: [libvirt] [PATCH] virsh: Allow graceful console shutdown

[Roman Bolshakov]

On Fri, Aug 23, 2019 at 03:33:36PM +0200, Michal Privoznik wrote:
> Currently, whenever there's a regular EOF on the console stream
> or an error the virStreamAbort() is called regardless. While this
> may not actually break anything, we should call virStreamFinish()
> to let the daemon know we've successfully received all the data
> and are shutting down the stream gracefully.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  tools/virsh-console.c | 41 +
>  1 file changed, 25 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/virsh-console.c b/tools/virsh-console.c
> index a235a9a283..900faa5087 100644
> --- a/tools/virsh-console.c
> +++ b/tools/virsh-console.c
> @@ -193,7 +202,7 @@ virConsoleEventOnStream(virStreamPtr st,
>  if (done == -2)
>  goto cleanup; /* blocking */
>  if (done < 0) {
> -virConsoleShutdown(con);
> +virConsoleShutdown(con, done == 0);
>  goto cleanup;
>  }
>  memmove(con->terminalToStream.data,

done is always non-zero here. If that's expected, a constant would be a
bit more cleaner here to sign explicit abort, rather than conditional.

Reviewed-by: Roman Bolshakov 

Thanks,
Roman

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

Re: [libvirt] [PATCH v4 27/42] libxl: introduce virtxend daemon

[Daniel P . Berrangé]

On Fri, Aug 23, 2019 at 05:11:18PM +, Jim Fehlig wrote:
> On 8/8/19 9:10 AM, Daniel P. Berrangé  wrote:
> > Arguably we could rename the libxl driver to "xen" since it is the
> > only xen driver we have these days, and that matches how we expose it
> > to users in the URI naming.
> 
> While thinking about this today I realized it is actually quite a rat hole. I 
> started by making a list of tasks associated with the rename
> 
> - Replace 'libxl' with 'xen' in the docs
> - Change configure option to '--with-xen' and drop '--with-libxl'
> - Rename virt-driver-libxl.m4 to virt-driver-xen.m4
> - Rename src/libxl to src/xen
> - Rename src/xen/libxl_*.[ch] to src/xen/xen_*.[ch}
> - Rename all the libxl files under tests/
> 
> These are easy enough, but should the list continue with renaming functions 
> in 
> all those files? And what about things that are not so easy to rename, e.g. 
> runtime directories such as /etc/libvirt/libxl, /var/lib/libvirt/libxl/, 
> /var/log/libvirt/libxl, etc. Would renaming some things but leaving others 
> cause 
> more confusion than the current situation?

Yeah the directory paths is where it gets "interesting", as we would need
to move files during the upgrade process. We've done that once before when
we moved from $HOME/.libvirt to the XDG directory layout. We had a startup
method in libvirtdm which i finally killed recently in
e10310d641365c83f4588670ac57e93d032db7f4

The libxl conversion would need to move more directories.

Where it might get especially painful is if there are things you cannot move
while VMs are running ?

I don't have a strong opinion either way - do whatever level of renaming
makes most sense to you.

> Regardless of how far to go with renaming libxl to xen, I think moving the 
> xen<->libvirt config converter files from src/xenconfig to src/libxl and 
> nuking 
> the xenconfig directory is a worthy endeavor.

Sure, makes sense.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH v4 27/42] libxl: introduce virtxend daemon

[Jim Fehlig]

On 8/8/19 9:10 AM, Daniel P. Berrangé  wrote:
> Arguably we could rename the libxl driver to "xen" since it is the
> only xen driver we have these days, and that matches how we expose it
> to users in the URI naming.

While thinking about this today I realized it is actually quite a rat hole. I 
started by making a list of tasks associated with the rename

- Replace 'libxl' with 'xen' in the docs
- Change configure option to '--with-xen' and drop '--with-libxl'
- Rename virt-driver-libxl.m4 to virt-driver-xen.m4
- Rename src/libxl to src/xen
- Rename src/xen/libxl_*.[ch] to src/xen/xen_*.[ch}
- Rename all the libxl files under tests/

These are easy enough, but should the list continue with renaming functions in 
all those files? And what about things that are not so easy to rename, e.g. 
runtime directories such as /etc/libvirt/libxl, /var/lib/libvirt/libxl/, 
/var/log/libvirt/libxl, etc. Would renaming some things but leaving others 
cause 
more confusion than the current situation?

Regardless of how far to go with renaming libxl to xen, I think moving the 
xen<->libvirt config converter files from src/xenconfig to src/libxl and nuking 
the xenconfig directory is a worthy endeavor.

Regards,
Jim

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

Re: [libvirt] [PATCH v2 7/9] qemu: add helper for getting full FSInfo

[Daniel Henrique Barboza]

On 8/23/19 11:00 AM, Jonathon Jongsma wrote:

On Thu, 2019-08-22 at 19:02 -0300, Daniel Henrique Barboza wrote:
[...]
Hmm, sorry for the sloppiness. I ran these tests multiple times, but
apparently after a final rebase, I introduced something that I failed
to catch before submitting. Thanks for testing it out.


No problem. I'll wait for your next version and resume testing - I am
interested in seeing this feature in action in PowerPC guests.


Thanks,


DHB



Jonathon




On 8/21/19 7:15 PM, Jonathon Jongsma wrote:

This function adds the complete filesystem information returned by
the
qemu agent to an array of typed parameters with field names
intended to
to be returned by virDomainGetGuestInfo()
---
  src/qemu/qemu_agent.c |  89 ++
  src/qemu/qemu_agent.h |   5 +
  tests/qemuagenttest.c | 210
+++---
  3 files changed, 291 insertions(+), 13 deletions(-)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index d5519cb243..c101805b23 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1953,6 +1953,95 @@ qemuAgentGetFSInfo(qemuAgentPtr mon,
virDomainFSInfoPtr **info,
  return ret;
  }
  
+int

+qemuAgentGetFSInfoParams(qemuAgentPtr mon,
+ virTypedParameterPtr *params,
+ int *nparams, int *maxparams,
+ virDomainDefPtr vmdef)
+{
+int ret = -1;
+qemuAgentFSInfoPtr *fsinfo = NULL;
+size_t i, j;
+int nfs;
+
+nfs = qemuAgentGetFSInfoInternal(mon, , vmdef);
+
+if (virTypedParamsAddUInt(params, nparams, maxparams,
+  "fs.count", nfs) < 0)
+goto cleanup;
+
+for (i = 0; i < nfs; i++) {
+char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.name", i);
+if (virTypedParamsAddString(params, nparams, maxparams,
+param_name, fsinfo[i]->name) <
0)
+goto cleanup;
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.mountpoint", i);
+if (virTypedParamsAddString(params, nparams, maxparams,
+param_name, fsinfo[i]-

mountpoint) < 0)

+goto cleanup;
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.fstype", i);
+if (virTypedParamsAddString(params, nparams, maxparams,
+param_name, fsinfo[i]->fstype)
< 0)
+goto cleanup;
+
+/* disk usage values are not returned by older guest
agents, so
+ * only add the params if the value is set */
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.total-bytes", i);
+if (fsinfo[i]->total_bytes != -1 &&
+virTypedParamsAddULLong(params, nparams, maxparams,
+param_name, fsinfo[i]-

total_bytes) < 0)

+goto cleanup;
+
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.used-bytes", i);
+if (fsinfo[i]->used_bytes != -1 &&
+virTypedParamsAddULLong(params, nparams, maxparams,
+param_name, fsinfo[i]-

used_bytes) < 0)

+goto cleanup;
+
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.disk.count", i);
+if (virTypedParamsAddUInt(params, nparams, maxparams,
+  param_name, fsinfo[i]->ndisks) <
0)
+goto cleanup;
+for (j = 0; j < fsinfo[i]->ndisks; j++) {
+qemuAgentDiskInfoPtr d = fsinfo[i]->disks[j];
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.disk.%zu.alias", i, j);
+if (d->alias &&
+virTypedParamsAddString(params, nparams,
maxparams,
+param_name, d->alias) < 0)
+goto cleanup;
+
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.disk.%zu.serial", i, j);
+if (d->serial &&
+virTypedParamsAddString(params, nparams,
maxparams,
+param_name, d->serial) <
0)
+goto cleanup;
+
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.disk.%zu.device", i, j);
+if (d->devnode &&
+virTypedParamsAddString(params, nparams,
maxparams,
+param_name, d->devnode) <
0)
+goto cleanup;
+}
+}
+ret = nfs;
+
+ cleanup:
+for (i = 0; i < nfs; i++)
+qemuAgentFSInfoFree(fsinfo[i]);
+VIR_FREE(fsinfo);
+
+return ret;
+}
  
  static int

  qemuAgentGetFSInfoInternal(qemuAgentPtr mon, qemuAgentFSInfoPtr
**info,
diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
index 

Re: [libvirt] [PATCH v2 03/16] domain: add rendernode attribute on

[Cole Robinson]

On 8/23/19 12:21 PM, Cole Robinson wrote:
> From: Marc-André Lureau 
> 
> vhost-user-gpu helper may accept --render-node option to specify on
> which GPU should the renderning be done.
> 

What does it do if the user doesn't pass one? Pick one itself, or just
not use one somehow?

If it picks one, then we may need to treat this like we treat other
rendernode instances and autoallocate one if the user doesn't specify,
otherwise we won't be able to add the path to the cgroup for example, or
selinux label it if necessary. I'm not sure if that actually applies in
this case, but it's worth considering.

> (by comparison  rendernode is the target/display rendering)
> 
> Signed-off-by: Marc-André Lureau 
> Signed-off-by: Cole Robinson 

Also I see now that I accidentally signed off all these patches, that
was not intentional. Please strip those from v3

> ---
>  docs/formatdomain.html.in |  5 +
>  docs/schemas/domaincommon.rng |  5 +
>  src/conf/domain_conf.c| 18 +-
>  src/conf/domain_conf.h|  1 +
>  4 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index ec650fbe17..5a4807d937 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -7059,6 +7059,11 @@ qemu-kvm -net nic,model=? /dev/null
>  Enable 3D acceleration (for vbox driver
>  since 0.7.1, qemu driver
>  since 1.3.0)
> +
> +rendernode
> +Absolute path to a host's DRI device to be used for
> +rendering (for vhost-user driver only,  +class="since">since 5.5.0)
>  
>
>  
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index bac566855d..6e91fe6cef 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3644,6 +3644,11 @@
>
>  
>
> +  
> +
> +  
> +
> +  
>  
>
>  
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index f51575d57d..2cc055491d 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2832,6 +2832,8 @@ virDomainVideoDefClear(virDomainVideoDefPtr def)
>  
>  virDomainDeviceInfoClear(>info);
>  
> +if (def->accel)
> +VIR_FREE(def->accel->rendernode);
>  VIR_FREE(def->accel);
>  VIR_FREE(def->virtio);
>  VIR_FREE(def->driver);
> @@ -15270,6 +15272,7 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
>  int val;
>  VIR_AUTOFREE(char *) accel2d = NULL;
>  VIR_AUTOFREE(char *) accel3d = NULL;
> +VIR_AUTOFREE(char *) rendernode = NULL;
>  
>  cur = node->children;
>  while (cur != NULL) {
> @@ -15278,12 +15281,13 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
>  virXMLNodeNameEqual(cur, "acceleration")) {
>  accel3d = virXMLPropString(cur, "accel3d");
>  accel2d = virXMLPropString(cur, "accel2d");
> +rendernode = virXMLPropString(cur, "rendernode");
>  }
>  }
>  cur = cur->next;
>  }
>  
> -if (!accel3d && !accel2d)
> +if (!accel3d && !accel2d && !rendernode)
>  return NULL;
>  
>  if (VIR_ALLOC(def) < 0)
> @@ -15307,6 +15311,9 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
>  def->accel2d = val;
>  }
>  
> +if (VIR_STRDUP(def->rendernode, rendernode) < 0)
> +goto cleanup;
> +

Huh, this function has no error reporting? A bogus accel2d/accel3d value
will raise an error but it never triggers the error path in the calling
function. Not your fault of course :)

>   cleanup:
>  return def;
>  }
> @@ -15474,6 +15481,11 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr 
> xmlopt,
>  goto error;
>  }
>  }
> +if (!def->vhostuser && def->accel && def->accel->rendernode) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +   _("unsupported rendernode accel attribute without 
> 'vhost-user'"));
> +goto error;
> +}
>  

This function doesn't represent best practices, but this style of check
should be moved to virDomainVideoDefValidate IMO

Thanks,
Cole

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

Re: [libvirt] [PATCH v2 02/16] domain: add "vhostuser" attribute to virtio video model

[Cole Robinson]

On 8/23/19 12:21 PM, Cole Robinson wrote:
> From: Marc-André Lureau 
> 
> Accept a new attribute to specify usage of helper process, ex:
> 
>   
> 
>   
> 

For other devices, we have





Which is an attempt to make this more generic. IMO using vhostuser='yes'
and is the simplest match to qemu terminology but maybe other people
have stronger opinions.

> Signed-off-by: Marc-André Lureau 
> Signed-off-by: Cole Robinson 
> ---
>  docs/formatdomain.html.in |  6 ++
>  docs/schemas/domaincommon.rng | 11 ++-
>  src/conf/domain_conf.c| 14 ++
>  src/conf/domain_conf.h|  1 +
>  4 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index fcb7c59c00..ec650fbe17 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -7039,6 +7039,12 @@ qemu-kvm -net nic,model=? /dev/null
>Attribute vram64 (since 
> 1.3.3)
>extends secondary bar and makes it addressable as 64bit memory.
>  
> +
> +  For guest type "kvm" and model type "virtio" there are
> +  optional attributes. Attribute vhost-user
> +  (since 5.5.0) specify that a
> +  vhost-user helper process should be associated with the GPU.
> +
>
>  
>acceleration
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index c48f8c4f56..bac566855d 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3581,7 +3581,6 @@
>  vmvga
>  xen
>  vbox
> -virtio
>  gop
>  none
>  bochs
> @@ -3607,6 +3606,16 @@
>  
>
>  
> +
> +  
> +virtio
> +  
> +  
> +
> +  
> +
> +  
> +
>
>
>  
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index b7a342bb91..f51575d57d 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -15358,6 +15358,7 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr 
> xmlopt,
>  xmlNodePtr cur;
>  VIR_XPATH_NODE_AUTORESTORE(ctxt);
>  VIR_AUTOFREE(char *) type = NULL;
> +VIR_AUTOFREE(char *) vhostuser = NULL;
>  VIR_AUTOFREE(char *) heads = NULL;
>  VIR_AUTOFREE(char *) vram = NULL;
>  VIR_AUTOFREE(char *) vram64 = NULL;
> @@ -15376,6 +15377,7 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr 
> xmlopt,
>  if (!type && !vram && !ram && !heads &&
>  virXMLNodeNameEqual(cur, "model")) {
>  type = virXMLPropString(cur, "type");
> +vhostuser = virXMLPropString(cur, "vhostuser");
>  ram = virXMLPropString(cur, "ram");
>  vram = virXMLPropString(cur, "vram");
>  vram64 = virXMLPropString(cur, "vram64");
> @@ -15408,6 +15410,16 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr 
> xmlopt,
>  def->type = virDomainVideoDefaultType(dom);
>  }
>  
> +if (vhostuser != NULL) {
> +if (virStringParseYesNo(vhostuser, >vhostuser) < 0) {
> +virReportError(VIR_ERR_INTERNAL_ERROR,
> +   _("unknown vhostuser value '%s'"), vhostuser);
> +goto cleanup;
> +}
> +} else {
> +def->vhostuser = false;
> +}
> +
>  if (ram) {
>  if (def->type != VIR_DOMAIN_VIDEO_TYPE_QXL) {
>  virReportError(VIR_ERR_XML_ERROR, "%s",
> @@ -26486,6 +26498,8 @@ virDomainVideoDefFormat(virBufferPtr buf,
>  virBufferAsprintf(buf, " heads='%u'", def->heads);
>  if (def->primary)
>  virBufferAddLit(buf, " primary='yes'");
> +if (def->vhostuser)
> +virBufferAddLit(buf, " vhostuser='yes'");
>  if (def->accel) {
>  virBufferAddLit(buf, ">\n");
>  virBufferAdjustIndent(buf, 2);
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 33cef5b75c..bc2450f25e 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1424,6 +1424,7 @@ struct _virDomainVideoDef {
>  virDomainVideoDriverDefPtr driver;
>  virDomainDeviceInfo info;
>  virDomainVirtioOptionsPtr virtio;
> +bool vhostuser;

I assume you followed the example of 'primary', but this should be a
virTristateBool. Follow 'accel3d' parsing/formatting as an example

Thanks,
Cole

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

Re: [libvirt] [PATCH v2 00/16] Add vhost-user-gpu support

[Cole Robinson]

On 8/23/19 12:21 PM, Cole Robinson wrote:
> v1: https://www.redhat.com/archives/libvir-list/2019-June/msg00102.html
> 
> This is v2 of Marc-André's series with minor changes. I'm not taking over
> this series, I just fixed these as part of the patch rebase so I can review
> it :)
> 
> Changes since v1:
> - rebase to master
> - if test file build by dropping LDADDS usage
> - syntax-check issues:
> * use #pragma once
> * if () bracket issues
> * jump label indent issues
> * error message %s usage
> * size_t for loops
> 
> I didn't know much about vhost-user-gpu before this series, here's what
> I've learned.
> 
> vhost-user is a generic mechanism that allows implementing virtio device
> dataplane handling in a separate userspace process. vhost-user-gpu here
> notably moves the virgl 3d handling out of the main qemu process. The
> external process will be /usr/libexec/vhost-user-gpu, which comes from
> qemu.git contrib/vhost-user-gpu code, first released in qemu-4.1.
> 
> Part of this series deals with discovering the location on disk of the
> vhost-user-gpu binary, and what capabilities it provides. This uses a
> similar mechanism to firmware.json, described in qemu
> docs/interop/vhost-user.json
> 
> https://github.com/qemu/qemu/blob/master/docs/interop/vhost-user.json
> 
> qemu 4.1 ships a 50-qemu-gpu.json to match. I believe virtio-fs
> will use a similar mechanism when it lands in upstream qemu, as
> virtiofsd is a separate process that communicates with qemu over
> vhost-user.
> 
> For a bit more background on vhost-user-gpu process handling and
> the json interop motivation, here's some of the qemu discussion:
> 
> https://lists.nongnu.org/archive/html/qemu-devel/2018-08/msg02610.html
> https://lists.nongnu.org/archive/html/qemu-devel/2018-09/msg00807.html
> 
> 
> For this series, the XML to enable this is:
> 
>   
> 
>   

That's wrong, it's

  

  

  

- Cole

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

Re: [libvirt] [PATCH v2 01/16] qemu: extract out qemuFetchConfigs from firmware

[Cole Robinson]

On 8/23/19 12:21 PM, Cole Robinson wrote:
> From: Marc-André Lureau 
> 
> The same config files disovery & priority rules are used for
> vhost-user backends.
> 
> Signed-off-by: Marc-André Lureau 
> Signed-off-by: Cole Robinson 
> ---
>  src/qemu/Makefile.inc.am |   2 +
>  src/qemu/qemu_configs.c  | 183 +++
>  src/qemu/qemu_configs.h  |  28 ++
>  src/qemu/qemu_firmware.c | 144 +-
>  4 files changed, 215 insertions(+), 142 deletions(-)
>  create mode 100644 src/qemu/qemu_configs.c
>  create mode 100644 src/qemu/qemu_configs.h
> 
> diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
> index 30a9751cfd..f7a0fa4a84 100644
> --- a/src/qemu/Makefile.inc.am
> +++ b/src/qemu/Makefile.inc.am
> @@ -30,6 +30,8 @@ QEMU_DRIVER_SOURCES = \
>   qemu/qemu_hotplugpriv.h \
>   qemu/qemu_conf.c \
>   qemu/qemu_conf.h \
> + qemu/qemu_configs.c \
> + qemu/qemu_configs.h \
>   qemu/qemu_process.c \
>   qemu/qemu_process.h \
>   qemu/qemu_processpriv.h \

The code looks fine, but the 'configs' naming is too generic. I suggest
going verbose with it, qemu_interop_json.[ch]. Functions then should be
named qemuInteropJSONXXX

I think you could start the series with this, and qemu_vhost_user.c but
minus the one function that uses virDomainDef additions, and those two
bits could be applied independent of the rest of the series IMO

Thanks,
Cole

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

[libvirt] [PATCH v3 6/9] qemu: add support for new fields in FSInfo

[Jonathon Jongsma]

Since version 3.0, qemu has returned disk usage statistics in
guest-get-fsinfo. And since 3.1, it has returned information about the
disk serial number and device node of disks that are targeted by the
filesystem.

Unfortunately, the public API virDomainGetFSInfo() returns the
filesystem info using a virDomainFSInfo struct, and due to API/ABI
guaranteeds it cannot be extended. So this new information cannot
easily be added to the public API. However, it is possible to add this
new filesystem information to a new virDomainGetGuestInfo() API which
will be based on typed parameters and is thus more extensible.

In order to support these two use cases, I added an internal struct
which the agent code uses to return all of the new data fields. This
internal struct can be converted to the public struct at a cost of some
extra memory allocation.

In a following commit, this additional information will be used within
virDomainGetGuestInfo().

Signed-off-by: Jonathon Jongsma 
---
 src/qemu/qemu_agent.c | 203 +-
 1 file changed, 182 insertions(+), 21 deletions(-)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index e986204162..8d54979f89 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1827,19 +1827,144 @@ qemuAgentSetTime(qemuAgentPtr mon,
 return ret;
 }
 
+typedef struct _qemuAgentDiskInfo qemuAgentDiskInfo;
+typedef qemuAgentDiskInfo *qemuAgentDiskInfoPtr;
+struct _qemuAgentDiskInfo {
+char *alias;
+char *serial;
+char *devnode;
+};
+
+typedef struct _qemuAgentFSInfo qemuAgentFSInfo;
+typedef qemuAgentFSInfo *qemuAgentFSInfoPtr;
+struct _qemuAgentFSInfo {
+char *mountpoint; /* path to mount point */
+char *name;   /* device name in the guest (e.g. "sda1") */
+char *fstype; /* filesystem type */
+long long total_bytes;
+long long used_bytes;
+size_t ndisks;
+qemuAgentDiskInfoPtr *disks;
+};
+
+static void
+qemuAgentDiskInfoFree(qemuAgentDiskInfoPtr info)
+{
+if (!info)
+return;
+
+VIR_FREE(info->serial);
+VIR_FREE(info->alias);
+VIR_FREE(info->devnode);
+VIR_FREE(info);
+}
+
+static void
+qemuAgentFSInfoFree(qemuAgentFSInfoPtr info)
+{
+size_t i;
+
+if (!info)
+return;
+
+VIR_FREE(info->mountpoint);
+VIR_FREE(info->name);
+VIR_FREE(info->fstype);
+
+for (i = 0; i < info->ndisks; i++)
+qemuAgentDiskInfoFree(info->disks[i]);
+VIR_FREE(info->disks);
+
+VIR_FREE(info);
+}
+
+static virDomainFSInfoPtr
+qemuAgentFSInfoToPublic(qemuAgentFSInfoPtr agent)
+{
+virDomainFSInfoPtr ret = NULL;
+size_t n;
+
+if (VIR_ALLOC(ret) < 0)
+return NULL;
 
+if (VIR_STRDUP(ret->name, agent->name) < 0)
+goto error;
+if (VIR_STRDUP(ret->mountpoint, agent->mountpoint) < 0)
+goto error;
+if (VIR_STRDUP(ret->fstype, agent->fstype) < 0)
+goto error;
+
+ret->ndevAlias = agent->ndisks;
+
+if (ret->ndevAlias == 0)
+return ret;
+
+if (VIR_ALLOC_N(ret->devAlias, ret->ndevAlias) < 0)
+goto error;
+
+for (n = 0; n < ret->ndevAlias; n++) {
+if (VIR_STRDUP(ret->devAlias[n], agent->disks[n]->alias) < 0)
+goto error;
+}
+
+return ret;
+
+ error:
+virDomainFSInfoFree(ret);
+return NULL;
+}
+
+static int
+qemuAgentGetFSInfoInternal(qemuAgentPtr mon, qemuAgentFSInfoPtr **info,
+   virDomainDefPtr vmdef);
 int
 qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info,
virDomainDefPtr vmdef)
+{
+int ret = -1;
+qemuAgentFSInfoPtr *agentinfo = NULL;
+virDomainFSInfoPtr *info_ret = NULL;
+size_t i;
+int nfs;
+
+nfs = qemuAgentGetFSInfoInternal(mon, , vmdef);
+if (nfs < 0)
+return ret;
+if (VIR_ALLOC_N(info_ret, nfs) < 0)
+goto cleanup;
+
+for (i = 0; i < nfs; i++) {
+if (!(info_ret[i] = qemuAgentFSInfoToPublic(agentinfo[i])))
+goto cleanup;
+}
+
+*info = info_ret;
+info_ret = NULL;
+ret = nfs;
+
+ cleanup:
+for (i = 0; i < nfs; i++) {
+qemuAgentFSInfoFree(agentinfo[i]);
+/* if there was an error, free any memory we've allocated for the
+ * return value */
+if (info_ret)
+virDomainFSInfoFree(info_ret[i]);
+}
+return ret;
+}
+
+
+static int
+qemuAgentGetFSInfoInternal(qemuAgentPtr mon, qemuAgentFSInfoPtr **info,
+   virDomainDefPtr vmdef)
 {
 size_t i, j, k;
 int ret = -1;
-size_t ndata = 0, ndisk;
-char **alias;
+size_t ndata = 0;
 virJSONValuePtr cmd;
 virJSONValuePtr reply = NULL;
 virJSONValuePtr data;
-virDomainFSInfoPtr *info_ret = NULL;
+qemuAgentFSInfoPtr *info_ret = NULL;
 virPCIDeviceAddress pci_address;
 const char *result = NULL;
 
@@ -1915,6 +2040,33 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr 
**info,
 if (VIR_STRDUP(info_ret[i]->fstype, result) < 

[libvirt] [PATCH v3 8/9] qemu: Implement virDomainGetGuestInfo()

[Jonathon Jongsma]

Iimplements the new guest information API by querying requested
information via the guest agent.

Signed-off-by: Jonathon Jongsma 
---
 src/qemu/qemu_driver.c | 110 +
 1 file changed, 110 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1b051a9424..446266e66b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -23190,6 +23190,115 @@ qemuDomainGetLaunchSecurityInfo(virDomainPtr domain,
 return ret;
 }
 
+static unsigned int supportedGuestInfoTypes =
+VIR_DOMAIN_GUEST_INFO_USERS |
+VIR_DOMAIN_GUEST_INFO_OS |
+VIR_DOMAIN_GUEST_INFO_TIMEZONE |
+VIR_DOMAIN_GUEST_INFO_HOSTNAME |
+VIR_DOMAIN_GUEST_INFO_FILESYSTEM;
+
+static void
+qemuDomainGetGuestInfoCheckSupport(unsigned int *types)
+{
+if (*types == 0)
+*types = supportedGuestInfoTypes;
+
+*types = *types & supportedGuestInfoTypes;
+}
+
+static int
+qemuDomainGetGuestInfo(virDomainPtr dom,
+   unsigned int types,
+   virTypedParameterPtr *params,
+   int *nparams,
+   unsigned int flags)
+{
+virQEMUDriverPtr driver = dom->conn->privateData;
+virDomainObjPtr vm = NULL;
+qemuAgentPtr agent;
+int ret = -1;
+int rv = -1;
+int maxparams = 0;
+char *hostname = NULL;
+virDomainDefPtr def = NULL;
+virCapsPtr caps = NULL;
+unsigned int supportedTypes = types;
+
+virCheckFlags(0, ret);
+qemuDomainGetGuestInfoCheckSupport();
+
+if (!(vm = qemuDomObjFromDomain(dom)))
+goto cleanup;
+
+if (virDomainGetGuestInfoEnsureACL(dom->conn, vm->def) < 0)
+goto cleanup;
+
+if (qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_QUERY) < 0)
+goto cleanup;
+
+if (!qemuDomainAgentAvailable(vm, true))
+goto endjob;
+
+agent = qemuDomainObjEnterAgent(vm);
+
+/* Although the libvirt qemu driver supports all of these guest info types,
+ * some guest agents might be too old to support these commands. If these
+ * info categories were explicitly requested (i.e. 'types' is non-zero),
+ * abort and report an error on any failures, otherwise continue and return
+ * as much info as is supported by the guest agent. */
+if (supportedTypes & VIR_DOMAIN_GUEST_INFO_USERS) {
+if (qemuAgentGetUsers(agent, params, nparams, ) < 0 &&
+types != 0)
+goto exitagent;
+}
+if (supportedTypes & VIR_DOMAIN_GUEST_INFO_OS) {
+if (qemuAgentGetOSInfo(agent, params, nparams, ) < 0
+&& types != 0)
+goto exitagent;
+}
+if (supportedTypes & VIR_DOMAIN_GUEST_INFO_TIMEZONE) {
+if (qemuAgentGetTimezone(agent, params, nparams, ) < 0
+&& types != 0)
+goto exitagent;
+}
+if (supportedTypes & VIR_DOMAIN_GUEST_INFO_HOSTNAME) {
+if (qemuAgentGetHostname(agent, ) < 0) {
+if (types != 0)
+goto exitagent;
+} else {
+if (virTypedParamsAddString(params, nparams, , 
"hostname",
+hostname) < 0)
+goto exitagent;
+}
+}
+if (supportedTypes & VIR_DOMAIN_GUEST_INFO_FILESYSTEM) {
+if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+goto exitagent;
+
+if (!(def = virDomainDefCopy(vm->def, caps, driver->xmlopt, NULL, 
false)))
+goto exitagent;
+
+if (qemuAgentGetFSInfoParams(agent, params, nparams, , def) 
< 0 &&
+types != 0)
+goto exitagent;
+}
+
+rv = 0;
+
+ exitagent:
+qemuDomainObjExitAgent(vm, agent);
+
+ endjob:
+qemuDomainObjEndAgentJob(vm);
+
+ cleanup:
+virDomainObjEndAPI();
+virDomainDefFree(def);
+virObjectUnref(caps);
+VIR_FREE(hostname);
+return rv;
+}
+
 static virHypervisorDriver qemuHypervisorDriver = {
 .name = QEMU_DRIVER_NAME,
 .connectURIProbe = qemuConnectURIProbe,
@@ -23425,6 +23534,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
 .domainCheckpointLookupByName = qemuDomainCheckpointLookupByName, /* 5.6.0 
*/
 .domainCheckpointGetParent = qemuDomainCheckpointGetParent, /* 5.6.0 */
 .domainCheckpointDelete = qemuDomainCheckpointDelete, /* 5.6.0 */
+.domainGetGuestInfo = qemuDomainGetGuestInfo, /* 5.6.0 */
 };
 
 
-- 
2.21.0

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

[libvirt] [PATCH v3 1/9] lib: add virDomainGetGuestInfo()

[Jonathon Jongsma]

This API is intended to aggregate several guest agent information
queries and is ispired by stats API virDomainListGetStats(). It is
anticipated that this information will be provided by a guest agent
running within the domain.

Signed-off-by: Jonathon Jongsma 
---
 include/libvirt/libvirt-domain.h |  14 
 src/driver-hypervisor.h  |   8 +++
 src/libvirt-domain.c | 117 +++
 src/libvirt_public.syms  |   1 +
 4 files changed, 140 insertions(+)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
index f160ee88b5..22277b0a84 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -4902,4 +4902,18 @@ int virDomainGetLaunchSecurityInfo(virDomainPtr domain,
int *nparams,
unsigned int flags);
 
+typedef enum {
+VIR_DOMAIN_GUEST_INFO_USERS = (1 << 0), /* return active users */
+VIR_DOMAIN_GUEST_INFO_OS = (1 << 1), /* return OS information */
+VIR_DOMAIN_GUEST_INFO_TIMEZONE = (1 << 2), /* return timezone information 
*/
+VIR_DOMAIN_GUEST_INFO_HOSTNAME = (1 << 3), /* return hostname information 
*/
+VIR_DOMAIN_GUEST_INFO_FILESYSTEM = (1 << 4), /* return filesystem 
information */
+} virDomainGuestInfoTypes;
+
+int virDomainGetGuestInfo(virDomainPtr domain,
+  unsigned int types,
+  virTypedParameterPtr *params,
+  int *nparams,
+  unsigned int flags);
+
 #endif /* LIBVIRT_DOMAIN_H */
diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
index c1632ae4c6..58eb731e85 100644
--- a/src/driver-hypervisor.h
+++ b/src/driver-hypervisor.h
@@ -1359,6 +1359,13 @@ typedef int
 (*virDrvDomainCheckpointDelete)(virDomainCheckpointPtr checkpoint,
 unsigned int flags);
 
+typedef int
+(*virDrvDomainGetGuestInfo)(virDomainPtr domain,
+unsigned int types,
+virTypedParameterPtr *params,
+int *nparams,
+unsigned int flags);
+
 typedef struct _virHypervisorDriver virHypervisorDriver;
 typedef virHypervisorDriver *virHypervisorDriverPtr;
 
@@ -1617,4 +1624,5 @@ struct _virHypervisorDriver {
 virDrvDomainCheckpointLookupByName domainCheckpointLookupByName;
 virDrvDomainCheckpointGetParent domainCheckpointGetParent;
 virDrvDomainCheckpointDelete domainCheckpointDelete;
+virDrvDomainGetGuestInfo domainGetGuestInfo;
 };
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 2fe9bb8e91..ad68db7549 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -12212,6 +12212,123 @@ virDomainSetVcpu(virDomainPtr domain,
 return -1;
 }
 
+/**
+ * virDomainGetGuestInfo:
+ * @domain: pointer to domain object
+ * @types: types of information to return, binary-OR of virDomainGuestInfoTypes
+ * @params: location to store the guest info parameters
+ * @nparams: number of items in @params
+ * @flags: currently unused, set to 0
+ *
+ * Queries the guest agent for the various information about the guest system.
+ * The reported data depends on the guest agent implementation. the information
+ * is returned as an array of typed parameters containing the individual
+ * parameters. The parameter name for each information field consists of a
+ * dot-separated strign containing the name of the requested group followed by
+ * a group-specific description of the statistic value.
+ *
+ * The information groups are enabled using the @types parameter which is a
+ * binary-OR of enum virDomainGuestInfoTypes. The following groups are 
available
+ * (although not necessarily implemented for each hypervisor):
+ *
+ * VIR_DOMAIN_GUEST_INFO_USERS:
+ *  returns information about users that are currently logged in within the
+ *  guest domain. The typed parameter keys are in this format:
+ *
+ *  "user.count" - the number of active users on this domain as an
+ * unsigned int
+ *  "user..name - username of the user as a string
+ *  "user..domain - domain of the user as a string (may only be
+ *   present on certain guest types)
+ *  "user..login-time - the login time of a user in milliseconds
+ *   since the epoch as unsigned long long
+ *
+ * VIR_DOMAIN_GUEST_INFO_OS:
+ *  Return information about the operating system running within the guest. The
+ *  typed parameter keys are in this format:
+ *
+ *  "os.id" - a string identifying the operating system
+ *  "os.name" - the name of the operating system, suitable for presentation
+ *  to a user, as a string
+ *  "os.pretty-name" - a pretty name for the operating system, suitable for
+ * presentation to a user, as a string
+ *  "os.version" - the version of the operating system suitable for
+ *

[libvirt] [PATCH v3 4/9] qemu: add helper function for querying OS info

[Jonathon Jongsma]

This function queries the guest operating system information and adds
the returned information to an array of typed parameters with field
names intended to be returned in virDomainGetGuestInfo().

Signed-off-by: Jonathon Jongsma 
---
 src/qemu/qemu_agent.c |  53 ++
 src/qemu/qemu_agent.h |   1 +
 tests/qemuagenttest.c | 122 ++
 3 files changed, 176 insertions(+)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 963a4b9359..0fcc3fdabd 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -2331,3 +2331,56 @@ qemuAgentGetUsers(qemuAgentPtr mon,
 virJSONValueFree(reply);
 return ret;
 }
+
+int
+qemuAgentGetOSInfo(qemuAgentPtr mon,
+   virTypedParameterPtr *params,
+   int *nparams,
+   int *maxparams)
+{
+int ret = -1;
+virJSONValuePtr cmd;
+virJSONValuePtr reply = NULL;
+virJSONValuePtr data = NULL;
+const char *result;
+
+if (!(cmd = qemuAgentMakeCommand("guest-get-osinfo", NULL)))
+return -1;
+
+if (qemuAgentCommand(mon, cmd, , true,
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
+goto cleanup;
+
+if (!(data = virJSONValueObjectGetObject(reply, "return"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("guest-get-osinfo reply was missing return data"));
+goto cleanup;
+}
+
+#define OSINFO_ADD_PARAM(agent_string_, param_string_) \
+do { \
+if ((result = virJSONValueObjectGetString(data, agent_string_))) { \
+if (virTypedParamsAddString(params, nparams, maxparams, \
+param_string_, result) < 0) { \
+goto cleanup; \
+} \
+} \
+} while (0)
+OSINFO_ADD_PARAM("id", "os.id");
+OSINFO_ADD_PARAM("name", "os.name");
+OSINFO_ADD_PARAM("pretty-name", "os.pretty-name");
+OSINFO_ADD_PARAM("version", "os.version");
+OSINFO_ADD_PARAM("version-id", "os.version-id");
+OSINFO_ADD_PARAM("machine", "os.machine");
+OSINFO_ADD_PARAM("variant", "os.variant");
+OSINFO_ADD_PARAM("variant-id", "os.variant-id");
+OSINFO_ADD_PARAM("kernel-release", "os.kernel-release");
+OSINFO_ADD_PARAM("kernel-version", "os.kernel-version");
+
+ret = 0;
+
+ cleanup:
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
index 05621b521a..ee019455e5 100644
--- a/src/qemu/qemu_agent.h
+++ b/src/qemu/qemu_agent.h
@@ -122,3 +122,4 @@ int qemuAgentSetUserPassword(qemuAgentPtr mon,
  bool crypted);
 
 int qemuAgentGetUsers(qemuAgentPtr mon, virTypedParameterPtr *params, int 
*nparams, int *maxparams);
+int qemuAgentGetOSInfo(qemuAgentPtr mon, virTypedParameterPtr *params, int 
*nparams, int *maxparams);
diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
index f2936a59f0..489b77d403 100644
--- a/tests/qemuagenttest.c
+++ b/tests/qemuagenttest.c
@@ -1068,6 +1068,127 @@ testQemuAgentUsers(const void *data)
 return ret;
 }
 
+static const char testQemuAgentOSInfoResponse[] =
+"{\"return\": "
+"   {\"name\":\"CentOS Linux\", "
+"   \"kernel-release\":\"3.10.0-862.14.4.el7.x86_64\", "
+"   \"version\":\"7 (Core)\", "
+"   \"pretty-name\":\"CentOS Linux 7 (Core)\", "
+"   \"version-id\":\"7\", "
+"   \"kernel-version\":\"#1 SMP Wed Sep 26 15:12:11 UTC 2018\", "
+"   \"machine\":\"x86_64\", "
+"   \"id\":\"centos\"} "
+"}";
+
+static const char testQemuAgentOSInfoResponse2[] =
+"{\"return\": "
+"   {\"name\":\"Microsoft Windows\", "
+"   \"kernel-release\":\"7601\", "
+"   \"version\":\"Microsoft Windows 77\", "
+"   \"variant\":\"client\", "
+"   \"pretty-name\":\"Windows 7 Professional\", "
+"   \"version-id\":\"\", "
+"   \"variant-id\":\"client\", "
+"   \"kernel-version\":\"6.1\", "
+"   \"machine\":\"x86_64\", "
+"   \"id\":\"mswindows\"} "
+"}";
+
+static int
+testQemuAgentOSInfo(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt);
+virTypedParameterPtr params = NULL;
+int nparams = 0;
+int maxparams = 0;
+int ret = -1;
+
+if (!test)
+return -1;
+
+if (qemuMonitorTestAddAgentSyncResponse(test) < 0)
+goto cleanup;
+
+if (qemuMonitorTestAddItem(test, "guest-get-osinfo",
+   testQemuAgentOSInfoResponse) < 0)
+goto cleanup;
+
+/* get osinfo */
+if (qemuAgentGetOSInfo(qemuMonitorTestGetAgent(test),
+   , , ) < 0)
+goto cleanup;
+
+if (nparams != 8) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   "Expected 8 params, got %d", nparams);
+goto cleanup;
+}
+#define VALIDATE_PARAM(param_name_, expected_) \
+

[libvirt] [PATCH v3 9/9] virsh: add 'guestinfo' command

[Jonathon Jongsma]

The 'guestinfo' command uses the new virDomainGetGuestInfo() API to
query information about the specified domain and print it out for the
user. The output is modeled roughly on the 'domstats' command.

Signed-off-by: Jonathon Jongsma 
---
 tools/virsh-domain.c | 85 
 1 file changed, 85 insertions(+)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index ccda71d7e0..977783951d 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -14038,6 +14038,85 @@ cmdDomFSInfo(vshControl *ctl, const vshCmd *cmd)
 return ret;
 }
 
+/*
+ * "guestinfo" command
+ */
+static const vshCmdInfo info_guestinfo[] = {
+{.name = "help",
+ .data = N_("query information about the guest (via agent)")
+},
+{.name = "desc",
+ .data = N_("Use the guest agent to query various information from guest's 
"
+"point of view")
+},
+{.name = NULL}
+};
+
+static const vshCmdOptDef opts_guestinfo[] = {
+VIRSH_COMMON_OPT_DOMAIN_FULL(VIR_CONNECT_LIST_DOMAINS_ACTIVE),
+{.name = "user",
+ .type = VSH_OT_BOOL,
+ .help = N_("report active users"),
+},
+{.name = "os",
+ .type = VSH_OT_BOOL,
+ .help = N_("report operating system information"),
+},
+{.name = "timezone",
+ .type = VSH_OT_BOOL,
+ .help = N_("report timezone information"),
+},
+{.name = "hostname",
+ .type = VSH_OT_BOOL,
+ .help = N_("report hostname"),
+},
+{.name = "filesystem",
+ .type = VSH_OT_BOOL,
+ .help = N_("report filesystem information"),
+},
+{.name = NULL}
+};
+
+static bool
+cmdGuestInfo(vshControl *ctl, const vshCmd *cmd)
+{
+virDomainPtr dom;
+bool ret = false;
+virTypedParameterPtr params = NULL;
+int nparams = 0;
+size_t i;
+unsigned int types = 0;
+
+if (vshCommandOptBool(cmd, "user"))
+types |= VIR_DOMAIN_GUEST_INFO_USERS;
+if (vshCommandOptBool(cmd, "os"))
+types |= VIR_DOMAIN_GUEST_INFO_OS;
+if (vshCommandOptBool(cmd, "timezone"))
+types |= VIR_DOMAIN_GUEST_INFO_TIMEZONE;
+if (vshCommandOptBool(cmd, "hostname"))
+types |= VIR_DOMAIN_GUEST_INFO_HOSTNAME;
+if (vshCommandOptBool(cmd, "filesystem"))
+types |= VIR_DOMAIN_GUEST_INFO_FILESYSTEM;
+
+if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+return false;
+
+if (virDomainGetGuestInfo(dom, types, , , 0) < 0)
+goto cleanup;
+
+for (i = 0; i < nparams; i++) {
+char *str = vshGetTypedParamValue(ctl, [i]);
+vshPrint(ctl, "%-20s: %s\n", params[i].field, str);
+VIR_FREE(str);
+}
+
+ret = true;
+
+ cleanup:
+virshDomainFree(dom);
+return ret;
+}
+
 const vshCmdDef domManagementCmds[] = {
 {.name = "attach-device",
  .handler = cmdAttachDevice,
@@ -14653,5 +14732,11 @@ const vshCmdDef domManagementCmds[] = {
  .info = info_domblkthreshold,
  .flags = 0
 },
+{.name = "guestinfo",
+ .handler = cmdGuestInfo,
+ .opts = opts_guestinfo,
+ .info = info_guestinfo,
+ .flags = 0
+},
 {.name = NULL}
 };
-- 
2.21.0

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

[libvirt] [PATCH v3 3/9] qemu: add helper for getting guest users

[Jonathon Jongsma]

This function fetches the list of logged-in users from the qemu agent
and adds them to a list of typed parameters so that they can be used
internally in libvirt.

Also add some basic tests for the function.

Signed-off-by: Jonathon Jongsma 
---
 src/qemu/qemu_agent.c |  91 +++
 src/qemu/qemu_agent.h |   2 +
 tests/qemuagenttest.c | 168 ++
 3 files changed, 261 insertions(+)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 361db299a5..963a4b9359 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -2240,3 +2240,94 @@ qemuAgentSetUserPassword(qemuAgentPtr mon,
 VIR_FREE(password64);
 return ret;
 }
+
+int
+qemuAgentGetUsers(qemuAgentPtr mon,
+  virTypedParameterPtr *params,
+  int *nparams,
+  int *maxparams)
+{
+int ret = -1;
+size_t i;
+virJSONValuePtr cmd;
+virJSONValuePtr reply = NULL;
+virJSONValuePtr data = NULL;
+size_t ndata;
+const char *strvalue;
+
+if (!(cmd = qemuAgentMakeCommand("guest-get-users", NULL)))
+return -1;
+
+if (qemuAgentCommand(mon, cmd, , true,
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
+goto cleanup;
+
+if (!(data = virJSONValueObjectGetArray(reply, "return"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("guest-get-users reply was missing return data"));
+goto cleanup;
+}
+
+if (!virJSONValueIsArray(data)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Malformed guest-get-users data array"));
+goto cleanup;
+}
+
+ndata = virJSONValueArraySize(data);
+
+if (virTypedParamsAddUInt(params, nparams, maxparams,
+  "user.count", ndata) < 0)
+goto cleanup;
+
+for (i = 0; i < ndata; i++) {
+char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
+virJSONValuePtr entry = virJSONValueArrayGet(data, i);
+
+if (!entry) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("array element missing in guest-get-users return "
+ "value"));
+goto cleanup;
+}
+
+if (!(strvalue = virJSONValueObjectGetString(entry, "user"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("'user' missing in reply of guest-get-users"));
+goto cleanup;
+}
+
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, "user.%zu.name", i);
+if (virTypedParamsAddString(params, nparams, maxparams,
+param_name, strvalue) < 0)
+goto cleanup;
+
+/* 'domain' is only present for windows guests */
+if ((strvalue = virJSONValueObjectGetString(entry, "domain"))) {
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "user.%zu.domain", i);
+if (virTypedParamsAddString(params, nparams, maxparams,
+param_name, strvalue) < 0)
+goto cleanup;
+}
+
+double logintime;
+if (virJSONValueObjectGetNumberDouble(entry, "login-time", ) 
< 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("'login-time' missing in reply of 
guest-get-users"));
+goto cleanup;
+}
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "user.%zu.login-time", i);
+if (virTypedParamsAddULLong(params, nparams, maxparams,
+param_name, logintime * 1000) < 0)
+goto cleanup;
+}
+
+ret = ndata;
+
+ cleanup:
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
index 6ae9fe54da..05621b521a 100644
--- a/src/qemu/qemu_agent.h
+++ b/src/qemu/qemu_agent.h
@@ -120,3 +120,5 @@ int qemuAgentSetUserPassword(qemuAgentPtr mon,
  const char *user,
  const char *password,
  bool crypted);
+
+int qemuAgentGetUsers(qemuAgentPtr mon, virTypedParameterPtr *params, int 
*nparams, int *maxparams);
diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
index 2f79986207..f2936a59f0 100644
--- a/tests/qemuagenttest.c
+++ b/tests/qemuagenttest.c
@@ -902,6 +902,173 @@ testQemuAgentGetInterfaces(const void *data)
 return ret;
 }
 
+static const char testQemuAgentUsersResponse[] =
+"{\"return\": "
+"   ["
+"   {\"user\": \"test\","
+"\"login-time\": 1561739203.584038"
+"   },"
+"   {\"user\": \"test2\","
+"\"login-time\": 1561739229.190697"
+"   }"
+"   ]"
+"}";
+
+static const char testQemuAgentUsersResponse2[] =
+"{\"return\": "
+"   ["
+"   {\"user\": \"test\","
+

[libvirt] [PATCH v3 2/9] remote: implement virDomainGetGuestInfo

[Jonathon Jongsma]

Add daemon and client code to serialize/deserialize
virDomainGetGuestInfo().

Signed-off-by: Jonathon Jongsma 
---
 src/remote/remote_daemon_dispatch.c | 41 ++
 src/remote/remote_driver.c  | 53 +
 src/remote/remote_protocol.x| 21 +++-
 src/remote_protocol-structs | 12 +++
 4 files changed, 126 insertions(+), 1 deletion(-)

diff --git a/src/remote/remote_daemon_dispatch.c 
b/src/remote/remote_daemon_dispatch.c
index 1bd281dd6d..665d938a99 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -7650,3 +7650,44 @@ remoteSerializeDomainDiskErrors(virDomainDiskErrorPtr 
errors,
 }
 return -1;
 }
+
+static int
+remoteDispatchDomainGetGuestInfo(virNetServerPtr server ATTRIBUTE_UNUSED,
+ virNetServerClientPtr client,
+ virNetMessagePtr msg ATTRIBUTE_UNUSED,
+ virNetMessageErrorPtr rerr,
+ remote_domain_get_guest_info_args *args,
+ remote_domain_get_guest_info_ret *ret)
+{
+int rv = -1;
+struct daemonClientPrivate *priv = 
virNetServerClientGetPrivateData(client);
+virDomainPtr dom = NULL;
+virTypedParameterPtr params = NULL;
+int nparams = 0;
+
+if (!priv->conn) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
+goto cleanup;
+}
+
+if (!(dom = get_nonnull_domain(priv->conn, args->dom)))
+goto cleanup;
+
+if (virDomainGetGuestInfo(dom, args->types, , , 
args->flags) < 0)
+goto cleanup;
+
+if (virTypedParamsSerialize(params, nparams,
+(virTypedParameterRemotePtr *) 
>params.params_val,
+>params.params_len,
+VIR_TYPED_PARAM_STRING_OKAY) < 0)
+goto cleanup;
+
+rv = 0;
+
+ cleanup:
+if (rv < 0)
+virNetMessageSaveError(rerr);
+virObjectUnref(dom);
+
+return rv;
+}
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index daac506672..5ba144648a 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -8306,6 +8306,58 @@ remoteNetworkPortGetParameters(virNetworkPortPtr port,
 return rv;
 }
 
+static int
+remoteDomainGetGuestInfo(virDomainPtr dom,
+ unsigned int types,
+ virTypedParameterPtr *params,
+ int *nparams,
+ unsigned int flags)
+{
+int rv = -1;
+struct private_data *priv = dom->conn->privateData;
+remote_domain_get_guest_info_args args;
+remote_domain_get_guest_info_ret ret;
+
+remoteDriverLock(priv);
+
+make_nonnull_domain(, dom);
+
+args.types = types;
+args.flags = flags;
+
+memset(, 0, sizeof(ret));
+
+if (call(dom->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_GUEST_INFO,
+ (xdrproc_t)xdr_remote_domain_get_guest_info_args, (char *),
+ (xdrproc_t)xdr_remote_domain_get_guest_info_ret, (char *)) == 
-1)
+goto done;
+
+if (ret.params.params_len > REMOTE_DOMAIN_GUEST_INFO_PARAMS_MAX) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Too many params in guestinfo: %d for limit %d"),
+   ret.params.params_len, 
REMOTE_DOMAIN_GUEST_INFO_PARAMS_MAX);
+goto cleanup;
+}
+
+if (params) {
+if (virTypedParamsDeserialize((virTypedParameterRemotePtr) 
ret.params.params_val,
+  ret.params.params_len,
+  REMOTE_DOMAIN_GUEST_INFO_PARAMS_MAX,
+  params,
+  nparams) < 0)
+goto cleanup;
+}
+
+rv = 0;
+
+ cleanup:
+xdr_free((xdrproc_t)xdr_remote_domain_get_guest_info_ret,
+ (char *) );
+
+ done:
+remoteDriverUnlock(priv);
+return rv;
+}
 
 /* get_nonnull_domain and get_nonnull_network turn an on-wire
  * (name, uuid) pair into virDomainPtr or virNetworkPtr object.
@@ -8733,6 +8785,7 @@ static virHypervisorDriver hypervisor_driver = {
 .domainCheckpointLookupByName = remoteDomainCheckpointLookupByName, /* 
5.6.0 */
 .domainCheckpointGetParent = remoteDomainCheckpointGetParent, /* 5.6.0 */
 .domainCheckpointDelete = remoteDomainCheckpointDelete, /* 5.6.0 */
+.domainGetGuestInfo = remoteDomainGetGuestInfo, /* 5.6.0 */
 };
 
 static virNetworkDriver network_driver = {
diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x
index 118369e2b3..75c2bc69ff 100644
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -269,6 +269,9 @@ const REMOTE_NODE_SEV_INFO_MAX = 64;
 /* Upper limit on number of launch security information entries */
 const REMOTE_DOMAIN_LAUNCH_SECURITY_INFO_PARAMS_MAX = 64;
 
+/* Upper limit on number of 

[libvirt] [PATCH v3 5/9] qemu: add helper for querying timezone info

[Jonathon Jongsma]

This function queries timezone information within the guest and adds
the information to an array of typed parameters with field names
intended to be returned to virDomainGetGuestInfo()

Signed-off-by: Jonathon Jongsma 
---
 src/qemu/qemu_agent.c | 46 ++
 src/qemu/qemu_agent.h |  1 +
 tests/qemuagenttest.c | 76 +++
 3 files changed, 123 insertions(+)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 0fcc3fdabd..e986204162 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -2384,3 +2384,49 @@ qemuAgentGetOSInfo(qemuAgentPtr mon,
 virJSONValueFree(reply);
 return ret;
 }
+
+int
+qemuAgentGetTimezone(qemuAgentPtr mon,
+ virTypedParameterPtr *params,
+ int *nparams,
+ int *maxparams)
+{
+int ret = -1;
+virJSONValuePtr cmd;
+virJSONValuePtr reply = NULL;
+virJSONValuePtr data = NULL;
+const char *name;
+int offset;
+
+if (!(cmd = qemuAgentMakeCommand("guest-get-timezone", NULL)))
+return -1;
+
+if (qemuAgentCommand(mon, cmd, , true,
+ VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0)
+goto cleanup;
+
+if (!(data = virJSONValueObjectGetObject(reply, "return"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("guest-get-timezone reply was missing return data"));
+goto cleanup;
+}
+
+if ((name = virJSONValueObjectGetString(data, "zone")) == NULL)
+goto cleanup;
+if (virTypedParamsAddString(params, nparams, maxparams,
+"timezone.name", name) < 0)
+goto cleanup;
+
+if ((virJSONValueObjectGetNumberInt(data, "offset", )) < 0)
+goto cleanup;
+if (virTypedParamsAddInt(params, nparams, maxparams,
+ "timezone.offset", offset) < 0)
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
index ee019455e5..69b0176855 100644
--- a/src/qemu/qemu_agent.h
+++ b/src/qemu/qemu_agent.h
@@ -123,3 +123,4 @@ int qemuAgentSetUserPassword(qemuAgentPtr mon,
 
 int qemuAgentGetUsers(qemuAgentPtr mon, virTypedParameterPtr *params, int 
*nparams, int *maxparams);
 int qemuAgentGetOSInfo(qemuAgentPtr mon, virTypedParameterPtr *params, int 
*nparams, int *maxparams);
+int qemuAgentGetTimezone(qemuAgentPtr mon, virTypedParameterPtr *params, int 
*nparams, int *maxparams);
diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
index 489b77d403..0912a5f29a 100644
--- a/tests/qemuagenttest.c
+++ b/tests/qemuagenttest.c
@@ -1189,7 +1189,82 @@ testQemuAgentOSInfo(const void *data)
 return ret;
 }
 
+static const char testQemuAgentTimezoneResponse1[] =
+"{\"return\":{\"zone\":\"IST\",\"offset\":19800}}";
+static const char testQemuAgentTimezoneResponse2[] =
+"{\"return\":{\"zone\":\"CEST\",\"offset\":7200}}";
+static const char testQemuAgentTimezoneResponse3[] =
+"{\"return\":{\"zone\":\"NDT\",\"offset\":-9000}}";
+static const char testQemuAgentTimezoneResponse4[] =
+"{\"return\":{\"zone\":\"PDT\",\"offset\":-25200}}";
 
+static int
+testQemuAgentTimezone(const void *data)
+{
+virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data;
+qemuMonitorTestPtr test = qemuMonitorTestNewAgent(xmlopt);
+int ret = -1;
+
+if (!test)
+return -1;
+
+#define VALIDATE_TIMEZONE(response_, expected_name_, expected_offset_) \
+do { \
+virTypedParameterPtr params_ = NULL; \
+int nparams_ = 0; \
+int maxparams_ = 0; \
+const char *name_ = NULL; \
+int offset_; \
+if (qemuMonitorTestAddAgentSyncResponse(test) < 0) \
+goto cleanup; \
+if (qemuMonitorTestAddItem(test, "guest-get-timezone", \
+   response_) < 0) \
+goto cleanup; \
+if (qemuAgentGetTimezone(qemuMonitorTestGetAgent(test), \
+ _, _, _) < 0) \
+goto cleanup; \
+if (nparams_ != 2) { \
+virReportError(VIR_ERR_INTERNAL_ERROR, \
+   "Expected 2 params, got %d", nparams_); \
+goto cleanup; \
+} \
+if (virTypedParamsGetString(params_, nparams_, \
+"timezone.name", _) < 0) { \
+virReportError(VIR_ERR_INTERNAL_ERROR, "missing param '%s'", \
+   "tiemzone.name"); \
+goto cleanup; \
+} \
+if (STRNEQ(name_, expected_name_)) { \
+virReportError(VIR_ERR_INTERNAL_ERROR, \
+   "Expected name '%s', got '%s'", expected_name_, 
name_); \
+goto cleanup; \
+} \
+if (virTypedParamsGetInt(params_, nparams_, \
+ "timezone.offset", _) < 0) { \
+

[libvirt] [PATCH v3 7/9] qemu: add helper for getting full FSInfo

[Jonathon Jongsma]

This function adds the complete filesystem information returned by the
qemu agent to an array of typed parameters with field names intended to
to be returned by virDomainGetGuestInfo()

Signed-off-by: Jonathon Jongsma 
---
 src/qemu/qemu_agent.c |  90 ++
 src/qemu/qemu_agent.h |   5 +
 tests/qemuagenttest.c | 210 +++---
 3 files changed, 292 insertions(+), 13 deletions(-)

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 8d54979f89..169ad74f6f 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1953,6 +1953,96 @@ qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr 
**info,
 return ret;
 }
 
+int
+qemuAgentGetFSInfoParams(qemuAgentPtr mon,
+ virTypedParameterPtr *params,
+ int *nparams, int *maxparams,
+ virDomainDefPtr vmdef)
+{
+int ret = -1;
+qemuAgentFSInfoPtr *fsinfo = NULL;
+size_t i, j;
+int nfs;
+
+if ((nfs = qemuAgentGetFSInfoInternal(mon, , vmdef)) < 0)
+return -1;
+
+if (virTypedParamsAddUInt(params, nparams, maxparams,
+  "fs.count", nfs) < 0)
+goto cleanup;
+
+for (i = 0; i < nfs; i++) {
+char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.name", i);
+if (virTypedParamsAddString(params, nparams, maxparams,
+param_name, fsinfo[i]->name) < 0)
+goto cleanup;
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.mountpoint", i);
+if (virTypedParamsAddString(params, nparams, maxparams,
+param_name, fsinfo[i]->mountpoint) < 0)
+goto cleanup;
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.fstype", i);
+if (virTypedParamsAddString(params, nparams, maxparams,
+param_name, fsinfo[i]->fstype) < 0)
+goto cleanup;
+
+/* disk usage values are not returned by older guest agents, so
+ * only add the params if the value is set */
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.total-bytes", i);
+if (fsinfo[i]->total_bytes != -1 &&
+virTypedParamsAddULLong(params, nparams, maxparams,
+param_name, fsinfo[i]->total_bytes) < 0)
+goto cleanup;
+
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.used-bytes", i);
+if (fsinfo[i]->used_bytes != -1 &&
+virTypedParamsAddULLong(params, nparams, maxparams,
+param_name, fsinfo[i]->used_bytes) < 0)
+goto cleanup;
+
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.disk.count", i);
+if (virTypedParamsAddUInt(params, nparams, maxparams,
+  param_name, fsinfo[i]->ndisks) < 0)
+goto cleanup;
+for (j = 0; j < fsinfo[i]->ndisks; j++) {
+qemuAgentDiskInfoPtr d = fsinfo[i]->disks[j];
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.disk.%zu.alias", i, j);
+if (d->alias &&
+virTypedParamsAddString(params, nparams, maxparams,
+param_name, d->alias) < 0)
+goto cleanup;
+
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.disk.%zu.serial", i, j);
+if (d->serial &&
+virTypedParamsAddString(params, nparams, maxparams,
+param_name, d->serial) < 0)
+goto cleanup;
+
+snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
+ "fs.%zu.disk.%zu.device", i, j);
+if (d->devnode &&
+virTypedParamsAddString(params, nparams, maxparams,
+param_name, d->devnode) < 0)
+goto cleanup;
+}
+}
+ret = nfs;
+
+ cleanup:
+for (i = 0; i < nfs; i++)
+qemuAgentFSInfoFree(fsinfo[i]);
+VIR_FREE(fsinfo);
+
+return ret;
+}
 
 static int
 qemuAgentGetFSInfoInternal(qemuAgentPtr mon, qemuAgentFSInfoPtr **info,
diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h
index 69b0176855..f6d74a2603 100644
--- a/src/qemu/qemu_agent.h
+++ b/src/qemu/qemu_agent.h
@@ -74,6 +74,11 @@ int qemuAgentFSThaw(qemuAgentPtr mon);
 int qemuAgentGetFSInfo(qemuAgentPtr mon, virDomainFSInfoPtr **info,
virDomainDefPtr vmdef);
 
+int qemuAgentGetFSInfoParams(qemuAgentPtr mon,
+ virTypedParameterPtr *params,
+ int *nparams, int *maxparams,
+ virDomainDefPtr 

[libvirt] [PATCH v3 0/9] add virDomainGetGuestInfo()

[Jonathon Jongsma]

changes in v3:
- fixed test failure
- fixed syntax issues that I had missed since I forgot to install cppi on my
  new laptop

This series adds several bits of guest information provided by a new API
function virDomainGetGuestInfo(). There is an implementation for qemu using the
guest agent. In particular, it adds information about logged-in users, guest
OS, timezone, hostname, and filesystem info.

Filesystem information is already accessible via the public API
virDomainGetFSInfo(), but recent versions of the qemu guest agent added
additional information, and the existing public API is not able to be extended
without breaking API since it returns a C struct. This new API uses typed
parameters so that it can be extended as necessary when new fields are added.

The overall API follows the bulk stats API quite closely, and tries to return
data in similar ways (for example, the "users.N.*" field name scheme was
inspired by various stats fields).

It should be noted that like version 1 of this patch series, the API still only
operates on a single domain, not a list of domains. I'm willing to consider
changing the API to operate on a list of domains if that is the concensus, but
I lean toward keeping it simpler.

Here's an example of the output of the virsh command added in this patch
operating on a fedora 30 guest:
virsh # guestinfo fedora30
user.count  : 1
user.0.name : test
user.0.login-time   : 1566422940895
os.id   : fedora
os.name : Fedora
os.pretty-name  : Fedora 30 (Workstation Edition)
os.version  : 30 (Workstation Edition)
os.version-id   : 30
os.machine  : x86_64
os.variant  : Workstation Edition
os.variant-id   : workstation
os.kernel-release   : 5.2.7-200.fc30.x86_64
os.kernel-version   : #1 SMP Thu Aug 8 05:35:29 UTC 2019
timezone.name   : CDT
timezone.offset : -18000
hostname: myhostname
fs.count: 3
fs.0.name   : dm-0
fs.0.mountpoint : /
fs.0.fstype : ext4
fs.0.total-bytes: 25928306688
fs.0.used-bytes : 10762133504
fs.0.disk.count : 1
fs.0.disk.0.alias   : vda
fs.0.disk.0.serial  : 947684ABZ8639Q
fs.0.disk.0.device  : /dev/vda2
fs.1.name   : vda1
fs.1.mountpoint : /boot
fs.1.fstype : ext4
fs.1.total-bytes: 952840192
fs.1.used-bytes : 229019648
fs.1.disk.count : 1
fs.1.disk.0.alias   : vda
fs.1.disk.0.serial  : 947684ABZ8639Q
fs.1.disk.0.device  : /dev/vda1
fs.2.name   : md127
fs.2.mountpoint : /run/media/test/971b1edc-da61-4015-b465-560a9b1b6e9b
fs.2.fstype : ext4
fs.2.total-bytes: 1950134272
fs.2.used-bytes : 6291456
fs.2.disk.count : 2
fs.2.disk.0.alias   : vdb
fs.2.disk.0.device  : /dev/vdb1
fs.2.disk.1.alias   : vdc
fs.2.disk.1.device  : /dev/vdc1

In contrast, here is the output of the virsh command operating on a fedora24
guest:
virsh # guestinfo  fedora24
error: Reconnected to the hypervisor
fs.count: 2
fs.0.name   : dm-0
fs.0.mountpoint : /
fs.0.fstype : ext4
fs.0.disk.count : 1
fs.0.disk.0.alias   : vda
fs.1.name   : vda1
fs.1.mountpoint : /boot
fs.1.fstype : ext4
fs.1.disk.count : 1
fs.1.disk.0.alias   : vda

However, if you specifically request an info category that is not supported by
the guest agent:
virsh # guestinfo --user fedora24
error: internal error: unable to execute QEMU agent command 'guest-get-users': 
The command guest-get-users has not been found



Jonathon Jongsma (9):
  lib: add virDomainGetGuestInfo()
  remote: implement virDomainGetGuestInfo
  qemu: add helper for getting guest users
  qemu: add helper function for querying OS info
  qemu: add helper for querying timezone info
  qemu: add support for new fields in FSInfo
  qemu: add helper for getting full FSInfo
  qemu: Implement virDomainGetGuestInfo()
  virsh: add 'guestinfo' command

 include/libvirt/libvirt-domain.h|  14 +
 src/driver-hypervisor.h |   8 +
 src/libvirt-domain.c| 117 ++
 src/libvirt_public.syms |   1 +
 src/qemu/qemu_agent.c   | 483 ++-
 src/qemu/qemu_agent.h   |   9 +
 src/qemu/qemu_driver.c  | 110 ++
 src/remote/remote_daemon_dispatch.c |  41 ++
 src/remote/remote_driver.c  |  53 +++
 src/remote/remote_protocol.x|  21 +-
 src/remote_protocol-structs |  12 +
 tests/qemuagenttest.c   | 576 +++-
 tools/virsh-domain.c|  85 
 13 files changed, 1495 insertions(+), 35 deletions(-)

-- 
2.21.0

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

[libvirt] [PATCH v2 05/16] qemu: add vhost-user-gpu capabilities checks

[Cole Robinson]

From: Marc-André Lureau 

Those new devices are merged for QEMU 4.1.

Signed-off-by: Marc-André Lureau 
Signed-off-by: Cole Robinson 
---
 src/qemu/qemu_capabilities.c | 4 
 src/qemu/qemu_capabilities.h | 2 ++
 tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml | 2 ++
 3 files changed, 8 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 73300128ea..d27f54d16e 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -537,6 +537,8 @@ VIR_ENUM_IMPL(virQEMUCaps,
   /* 335 */
   "bochs-display",
   "migration-file-drop-cache",
+  "vhost-user-gpu",
+  "vhost-user-vga",
 );
 
 
@@ -1127,6 +1129,8 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
 { "virtio-serial-pci-non-transitional", QEMU_CAPS_VIRTIO_PCI_TRANSITIONAL 
},
 { "max-x86_64-cpu", QEMU_CAPS_X86_MAX_CPU },
 { "bochs-display", QEMU_CAPS_DEVICE_BOCHS_DISPLAY },
+{ "vhost-user-gpu", QEMU_CAPS_DEVICE_VHOST_USER_GPU },
+{ "vhost-user-vga", QEMU_CAPS_DEVICE_VHOST_USER_VGA },
 };
 
 static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsVirtioBalloon[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 68ef6c49b4..6f1e1dd2f3 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -518,6 +518,8 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 /* 335 */
 QEMU_CAPS_DEVICE_BOCHS_DISPLAY, /* -device bochs-display */
 QEMU_CAPS_MIGRATION_FILE_DROP_CACHE, /* migration with disk cache on is 
safe for type='file' disks */
+QEMU_CAPS_DEVICE_VHOST_USER_GPU, /* -device vhost-user-gpu */
+QEMU_CAPS_DEVICE_VHOST_USER_VGA, /* -device vhost-user-vga */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
index f4583d7fe7..5d4540b3f7 100644
--- a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
@@ -209,6 +209,8 @@
   
   
   
+  
+  
   450
   0
   43100759
-- 
2.21.0

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

[libvirt] [PATCH v2 09/16] qemu: add vhost-user helpers

[Cole Robinson]

From: Marc-André Lureau 

Add qemuVhostUserFetchConfigs() to discover vhost-user helpers.

qemuVhostUserFillDomainGPU() will find the first matching GPU helper
with the required capabilities and set the associated
vhost_user_binary.

Signed-off-by: Marc-André Lureau 
Signed-off-by: Cole Robinson 
---
 src/conf/device_conf.c|   1 +
 src/conf/device_conf.h|   1 +
 src/qemu/Makefile.inc.am  |   2 +
 src/qemu/qemu_vhost_user.c| 394 ++
 src/qemu/qemu_vhost_user.h|  48 +++
 tests/Makefile.am |   9 +
 .../etc/qemu/vhost-user/40-gpu.json   |   1 +
 .../etc/qemu/vhost-user/50-gpu.json   |   0
 .../qemu/vhost-user/test-vhost-user-gpu   |  11 +
 .../usr/share/qemu/vhost-user/30-gpu.json |   1 +
 .../usr/share/qemu/vhost-user/50-gpu.json |   8 +
 .../usr/share/qemu/vhost-user/60-gpu.json |   1 +
 tests/qemuvhostusertest.c | 132 ++
 13 files changed, 609 insertions(+)
 create mode 100644 src/qemu/qemu_vhost_user.c
 create mode 100644 src/qemu/qemu_vhost_user.h
 create mode 12 tests/qemuvhostuserdata/etc/qemu/vhost-user/40-gpu.json
 create mode 100644 tests/qemuvhostuserdata/etc/qemu/vhost-user/50-gpu.json
 create mode 100755 
tests/qemuvhostuserdata/usr/libexec/qemu/vhost-user/test-vhost-user-gpu
 create mode 12 
tests/qemuvhostuserdata/usr/share/qemu/vhost-user/30-gpu.json
 create mode 100644 
tests/qemuvhostuserdata/usr/share/qemu/vhost-user/50-gpu.json
 create mode 12 
tests/qemuvhostuserdata/usr/share/qemu/vhost-user/60-gpu.json
 create mode 100644 tests/qemuvhostusertest.c

diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index 4c57f0995f..2f7077ddc4 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -96,6 +96,7 @@ virDomainDeviceInfoClear(virDomainDeviceInfoPtr info)
 VIR_FREE(info->loadparm);
 info->isolationGroup = 0;
 info->isolationGroupLocked = false;
+VIR_FREE(info->vhost_user_binary);
 }
 
 void
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index d0854925e3..0b0c525ad8 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -179,6 +179,7 @@ struct _virDomainDeviceInfo {
  * cases we might want to prevent that from happening by
  * locking the isolation group */
 bool isolationGroupLocked;
+char *vhost_user_binary;
 };
 
 void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info);
diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
index f7a0fa4a84..18a9220d01 100644
--- a/src/qemu/Makefile.inc.am
+++ b/src/qemu/Makefile.inc.am
@@ -60,6 +60,8 @@ QEMU_DRIVER_SOURCES = \
qemu/qemu_qapi.h \
qemu/qemu_tpm.c \
qemu/qemu_tpm.h \
+   qemu/qemu_vhost_user.c \
+   qemu/qemu_vhost_user.h \
$(NULL)
 
 
diff --git a/src/qemu/qemu_vhost_user.c b/src/qemu/qemu_vhost_user.c
new file mode 100644
index 00..7a97052ab9
--- /dev/null
+++ b/src/qemu/qemu_vhost_user.c
@@ -0,0 +1,394 @@
+/*
+ * qemu_vhost_user.c: QEMU vhost-user
+ *
+ * Copyright (C) 2019 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
+ * .
+ */
+
+#include 
+
+#include "qemu_vhost_user.h"
+#include "qemu_configs.h"
+#include "virjson.h"
+#include "virlog.h"
+#include "virstring.h"
+#include "viralloc.h"
+#include "virenum.h"
+
+#define VIR_FROM_THIS VIR_FROM_QEMU
+
+VIR_LOG_INIT("qemu.qemu_vhost_user");
+
+typedef enum {
+QEMU_VHOST_USER_TYPE_NONE = 0,
+QEMU_VHOST_USER_TYPE_9P,
+QEMU_VHOST_USER_TYPE_BALLOON,
+QEMU_VHOST_USER_TYPE_BLOCK,
+QEMU_VHOST_USER_TYPE_CAIF,
+QEMU_VHOST_USER_TYPE_CONSOLE,
+QEMU_VHOST_USER_TYPE_CRYPTO,
+QEMU_VHOST_USER_TYPE_GPU,
+QEMU_VHOST_USER_TYPE_INPUT,
+QEMU_VHOST_USER_TYPE_NET,
+QEMU_VHOST_USER_TYPE_RNG,
+QEMU_VHOST_USER_TYPE_RPMSG,
+QEMU_VHOST_USER_TYPE_RPROC_SERIAL,
+QEMU_VHOST_USER_TYPE_SCSI,
+QEMU_VHOST_USER_TYPE_VSOCK,
+
+QEMU_VHOST_USER_TYPE_LAST
+} qemuVhostUserType;
+
+VIR_ENUM_DECL(qemuVhostUserType);
+VIR_ENUM_IMPL(qemuVhostUserType,
+  QEMU_VHOST_USER_TYPE_LAST,
+  "",
+  "9p",
+  "balloon",
+  "block",
+  "caif",
+  "console",
+  "crypto",
+  "gpu",
+

[libvirt] [PATCH v2 14/16] qemu: build vhost-user GPU devices

[Cole Robinson]

From: Marc-André Lureau 

For each vhost-user GPUs,
- build a socket chardev, and pass the vhost-user socket to it
- build a vhost-user video device and associate it with the chardev

Signed-off-by: Marc-André Lureau 
Signed-off-by: Cole Robinson 
---
 src/qemu/qemu_command.c | 46 +++--
 1 file changed, 44 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 8bef103f68..0e1d9510e5 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4672,8 +4672,15 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
 goto error;
 }
 
-if (STREQ(model, "virtio-gpu")) {
-if (qemuBuildVirtioDevStr(, "virtio-gpu", qemuCaps,
+if (video->vhostuser) {
+if (STREQ(model, "virtio-vga"))
+model = "vhost-user-vga";
+if (STREQ(model, "virtio-gpu"))
+model = "vhost-user-gpu";
+}
+
+if (STREQ(model, "virtio-gpu") || STREQ(model, "vhost-user-gpu")) {
+if (qemuBuildVirtioDevStr(, model, qemuCaps,
   VIR_DOMAIN_DEVICE_VIDEO, video) < 0) {
 goto error;
 }
@@ -4715,6 +4722,10 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
 if (video->heads)
 virBufferAsprintf(, ",max_outputs=%u", video->heads);
 }
+} else if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && 
video->vhostuser) {
+if (video->heads)
+virBufferAsprintf(, ",max_outputs=%u", video->heads);
+virBufferAsprintf(, ",chardev=chr-vu-%s", video->info.alias);
 } else if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO) {
 if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_GPU_MAX_OUTPUTS)) {
 if (video->heads)
@@ -4830,6 +4841,23 @@ qemuBuildVgaVideoCommand(virCommandPtr cmd,
 }
 
 
+static char *
+qemuBuildVhostUserChardevStr(const char *alias,
+ int *fd,
+ virCommandPtr cmd)
+{
+char *chardev = NULL;
+
+if (virAsprintf(, "socket,id=chr-vu-%s,fd=%d", alias, *fd) < 0)
+return NULL;
+
+virCommandPassFD(cmd, *fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+*fd = -1;
+
+return chardev;
+}
+
+
 static int
 qemuBuildVideoCommandLine(virCommandPtr cmd,
   const virDomainDef *def,
@@ -4837,6 +4865,20 @@ qemuBuildVideoCommandLine(virCommandPtr cmd,
 {
 size_t i;
 
+for (i = 0; i < def->nvideos; i++) {
+VIR_AUTOFREE(char *) chardev = NULL;
+virDomainVideoDefPtr video = def->videos[i];
+
+if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && video->vhostuser) {
+if (!(chardev = qemuBuildVhostUserChardevStr(video->info.alias,
+ 
>info.vhost_user_fd,
+ cmd)))
+return -1;
+
+virCommandAddArgList(cmd, "-chardev", chardev, NULL);
+}
+}
+
 for (i = 0; i < def->nvideos; i++) {
 char *str = NULL;
 virDomainVideoDefPtr video = def->videos[i];
-- 
2.21.0

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

[libvirt] [PATCH v2 07/16] qemu: validate virtio-gpu with vhost-user

[Cole Robinson]

From: Marc-André Lureau 

Check qemu capability, and accept 3d acceleration. 3d acceleration
support is checked when looking for a suitable vhost-user helper.

Signed-off-by: Marc-André Lureau 
Signed-off-by: Cole Robinson 
---
 src/qemu/qemu_process.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index c9921646e9..c439f17011 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5274,8 +5274,10 @@ qemuProcessStartValidateVideo(virDomainObjPtr vm,
  !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMWARE_SVGA)) ||
 (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL &&
  !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QXL)) ||
-(video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO &&
+(video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && !video->vhostuser 
&&
  !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU)) ||
+(video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && video->vhostuser &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VHOST_USER_GPU)) ||
 (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO &&
  video->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW &&
  !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW))) {
@@ -5285,7 +5287,15 @@ qemuProcessStartValidateVideo(virDomainObjPtr vm,
 return -1;
 }
 
-if (video->accel) {
+if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO &&
+video->vhostuser &&
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VHOST_USER_GPU)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("this QEMU does not support vhost-user backed 
video device"));
+return -1;
+}
+
+if (!video->vhostuser && video->accel) {
 if (video->accel->accel3d == VIR_TRISTATE_SWITCH_ON &&
 (video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO ||
  !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_GPU_VIRGL))) {
-- 
2.21.0

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

[libvirt] [PATCH v2 11/16] qemu: add vhost-user-gpu helper unit

[Cole Robinson]

From: Marc-André Lureau 

Similar to the qemu_tpm.c, add a unit with a few functions to
start/stop and setup the cgroup of the external vhost-user-gpu
process. See function documentation.

Since the vhost-user connection fd isn't necessarily specific to QEMU,
it was easier to add it to virDomainDeviceInfo, although a reasonable
place could be qemuDomainObjPrivate (with an associate hashtable
device-info -> qemu-device-info for example).

Signed-off-by: Marc-André Lureau 
Signed-off-by: Cole Robinson 
---
 src/conf/device_conf.h |   1 +
 src/qemu/Makefile.inc.am   |   2 +
 src/qemu/qemu_vhost_user_gpu.c | 305 +
 src/qemu/qemu_vhost_user_gpu.h |  50 ++
 4 files changed, 358 insertions(+)
 create mode 100644 src/qemu/qemu_vhost_user_gpu.c
 create mode 100644 src/qemu/qemu_vhost_user_gpu.h

diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index 0b0c525ad8..23cc2e9ba6 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -180,6 +180,7 @@ struct _virDomainDeviceInfo {
  * locking the isolation group */
 bool isolationGroupLocked;
 char *vhost_user_binary;
+int vhost_user_fd;
 };
 
 void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info);
diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
index 18a9220d01..d394eab957 100644
--- a/src/qemu/Makefile.inc.am
+++ b/src/qemu/Makefile.inc.am
@@ -62,6 +62,8 @@ QEMU_DRIVER_SOURCES = \
qemu/qemu_tpm.h \
qemu/qemu_vhost_user.c \
qemu/qemu_vhost_user.h \
+   qemu/qemu_vhost_user_gpu.c \
+   qemu/qemu_vhost_user_gpu.h \
$(NULL)
 
 
diff --git a/src/qemu/qemu_vhost_user_gpu.c b/src/qemu/qemu_vhost_user_gpu.c
new file mode 100644
index 00..0735af1473
--- /dev/null
+++ b/src/qemu/qemu_vhost_user_gpu.c
@@ -0,0 +1,305 @@
+/*
+ * qemu_vhost_user_gpu.c: QEMU vhost-user GPU support
+ *
+ * Copyright (C) 2018 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
+ * .
+ */
+
+#include 
+
+#include "qemu_vhost_user_gpu.h"
+#include "qemu_vhost_user.h"
+#include "qemu_extdevice.h"
+
+#include "conf/domain_conf.h"
+#include "configmake.h"
+#include "vircommand.h"
+#include "viralloc.h"
+#include "virlog.h"
+#include "virutil.h"
+#include "virfile.h"
+#include "virstring.h"
+#include "virtime.h"
+#include "virpidfile.h"
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+VIR_LOG_INIT("qemu.vhost-user-gpu");
+
+
+static char *
+qemuVhostUserGPUCreatePidFilename(const char *stateDir,
+  const char *shortName,
+  const char *alias)
+{
+char *pidfile = NULL;
+char *devicename = NULL;
+
+if (virAsprintf(, "%s-%s-vhost-user-gpu", shortName, alias) < 0)
+return NULL;
+
+pidfile = virPidFileBuildPath(stateDir, devicename);
+
+VIR_FREE(devicename);
+
+return pidfile;
+}
+
+
+/*
+ * qemuVhostUserGPUGetPid:
+ * @binpath: path of executable associated with the pidfile
+ * @stateDir: the directory where vhost-user-gpu writes the pidfile into
+ * @shortName: short name of the domain
+ * @alias: video device alias
+ * @pid: pointer to pid
+ *
+ * Return -errno upon error, or zero on successful reading of the pidfile.
+ * If the PID was not still alive, zero will be returned, and @pid will be
+ * set to -1;
+ */
+static int
+qemuVhostUserGPUGetPid(const char *binPath,
+   const char *stateDir,
+   const char *shortName,
+   const char *alias,
+   pid_t *pid)
+{
+int ret;
+char *pidfile = qemuVhostUserGPUCreatePidFilename(stateDir, shortName, 
alias);
+if (!pidfile)
+return -ENOMEM;
+
+ret = virPidFileReadPathIfAlive(pidfile, pid, binPath);
+
+VIR_FREE(pidfile);
+
+return ret;
+}
+
+
+int qemuExtVhostUserGPUPrepareDomain(virQEMUDriverPtr driver,
+ virDomainVideoDefPtr video)
+{
+return qemuVhostUserFillDomainGPU(driver, video);
+}
+
+
+/*
+ * qemuExtVhostUserGPUStart:
+ * @driver: QEMU driver
+ * @vm: the VM domain
+ * @video: the video device
+ * @logCtxt: log context
+ *
+ * Start the external vhost-user-gpu process:
+ * - open a socketpair for vhost-user communication
+ * - have the command line built
+ * - start the external process and sync with it before QEMU start
+ */
+int 

[libvirt] [PATCH v2 08/16] qemu: restrict 'virgl=' option to non-vhostuser video type

[Cole Robinson]

From: Marc-André Lureau 

vhost-user device doesn't have a virgl option, it is passed to the
vhost-user-gpu helper process instead.

Signed-off-by: Marc-André Lureau 
Signed-off-by: Cole Robinson 
---
 src/qemu/qemu_command.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 49652a8565..8bef103f68 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4683,9 +4683,11 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
 
 virBufferAsprintf(, ",id=%s", video->info.alias);
 
-if (video->accel && video->accel->accel3d == VIR_TRISTATE_SWITCH_ON) {
-virBufferAsprintf(, ",virgl=%s",
-  
virTristateSwitchTypeToString(video->accel->accel3d));
+if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && !video->vhostuser) {
+if (video->accel && video->accel->accel3d == VIR_TRISTATE_SWITCH_ON) {
+virBufferAsprintf(, ",virgl=%s",
+  
virTristateSwitchTypeToString(video->accel->accel3d));
+}
 }
 
 if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL) {
-- 
2.21.0

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

[libvirt] [PATCH v2 04/16] qemu-cgroup: allow accel rendernode access

[Cole Robinson]

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
Signed-off-by: Cole Robinson 
---
 src/qemu/qemu_cgroup.c | 24 
 1 file changed, 24 insertions(+)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index ecd96efb0a..eb6f993d8e 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -503,6 +503,25 @@ qemuSetupGraphicsCgroup(virDomainObjPtr vm,
 }
 
 
+static int
+qemuSetupVideoAccelCgroup(virDomainObjPtr vm,
+  virDomainVideoAccelDefPtr def)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+int ret;
+
+if (!def->rendernode ||
+!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES))
+return 0;
+
+ret = virCgroupAllowDevicePath(priv->cgroup, def->rendernode,
+   VIR_CGROUP_DEVICE_RW, false);
+virDomainAuditCgroupPath(vm, priv->cgroup, "allow", def->rendernode,
+ "rw", ret);
+return ret;
+}
+
+
 static int
 qemuSetupBlkioCgroup(virDomainObjPtr vm)
 {
@@ -803,6 +822,11 @@ qemuSetupDevicesCgroup(virDomainObjPtr vm)
 goto cleanup;
 }
 
+for (i = 0; i < vm->def->nvideos; i++) {
+if (qemuSetupVideoAccelCgroup(vm, vm->def->videos[i]->accel) < 0)
+goto cleanup;
+}
+
 for (i = 0; i < vm->def->ninputs; i++) {
 if (qemuSetupInputCgroup(vm, vm->def->inputs[i]) < 0)
 goto cleanup;
-- 
2.21.0

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

[libvirt] [PATCH v2 06/16] qemu: check that qemu is vhost-user-vga capable

[Cole Robinson]

From: Marc-André Lureau 

To support virtio VGA with vhost-user, vhost-user-vga device is necessary.

Signed-off-by: Marc-André Lureau 
Signed-off-by: Cole Robinson 
---
 src/qemu/qemu_domain.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d0722f8761..bf0531126d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -12386,9 +12386,14 @@ bool
 qemuDomainSupportsVideoVga(virDomainVideoDefPtr video,
virQEMUCapsPtr qemuCaps)
 {
-if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO &&
-!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_VGA))
-return false;
+if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO) {
+if (video->vhostuser) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VHOST_USER_VGA))
+return false;
+} else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_VGA)) {
+return false;
+}
+}
 
 return true;
 }
-- 
2.21.0

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

[libvirt] [PATCH v2 03/16] domain: add rendernode attribute on

[Cole Robinson]

From: Marc-André Lureau 

vhost-user-gpu helper may accept --render-node option to specify on
which GPU should the renderning be done.

(by comparison  rendernode is the target/display rendering)

Signed-off-by: Marc-André Lureau 
Signed-off-by: Cole Robinson 
---
 docs/formatdomain.html.in |  5 +
 docs/schemas/domaincommon.rng |  5 +
 src/conf/domain_conf.c| 18 +-
 src/conf/domain_conf.h|  1 +
 4 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index ec650fbe17..5a4807d937 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -7059,6 +7059,11 @@ qemu-kvm -net nic,model=? /dev/null
 Enable 3D acceleration (for vbox driver
 since 0.7.1, qemu driver
 since 1.3.0)
+
+rendernode
+Absolute path to a host's DRI device to be used for
+rendering (for vhost-user driver only, since 5.5.0)
 
   
 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index bac566855d..6e91fe6cef 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3644,6 +3644,11 @@
   
 
   
+  
+
+  
+
+  
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f51575d57d..2cc055491d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2832,6 +2832,8 @@ virDomainVideoDefClear(virDomainVideoDefPtr def)
 
 virDomainDeviceInfoClear(>info);
 
+if (def->accel)
+VIR_FREE(def->accel->rendernode);
 VIR_FREE(def->accel);
 VIR_FREE(def->virtio);
 VIR_FREE(def->driver);
@@ -15270,6 +15272,7 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
 int val;
 VIR_AUTOFREE(char *) accel2d = NULL;
 VIR_AUTOFREE(char *) accel3d = NULL;
+VIR_AUTOFREE(char *) rendernode = NULL;
 
 cur = node->children;
 while (cur != NULL) {
@@ -15278,12 +15281,13 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
 virXMLNodeNameEqual(cur, "acceleration")) {
 accel3d = virXMLPropString(cur, "accel3d");
 accel2d = virXMLPropString(cur, "accel2d");
+rendernode = virXMLPropString(cur, "rendernode");
 }
 }
 cur = cur->next;
 }
 
-if (!accel3d && !accel2d)
+if (!accel3d && !accel2d && !rendernode)
 return NULL;
 
 if (VIR_ALLOC(def) < 0)
@@ -15307,6 +15311,9 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
 def->accel2d = val;
 }
 
+if (VIR_STRDUP(def->rendernode, rendernode) < 0)
+goto cleanup;
+
  cleanup:
 return def;
 }
@@ -15474,6 +15481,11 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt,
 goto error;
 }
 }
+if (!def->vhostuser && def->accel && def->accel->rendernode) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("unsupported rendernode accel attribute without 
'vhost-user'"));
+goto error;
+}
 
 if (virDomainDeviceInfoParseXML(xmlopt, node, >info, flags) < 0)
 goto error;
@@ -26452,6 +26464,10 @@ virDomainVideoAccelDefFormat(virBufferPtr buf,
 virBufferAsprintf(buf, " accel2d='%s'",
   virTristateBoolTypeToString(def->accel2d));
 }
+if (def->rendernode) {
+virBufferAsprintf(buf, " rendernode='%s'",
+  def->rendernode);
+}
 virBufferAddLit(buf, "/>\n");
 }
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index bc2450f25e..707fbd1cd3 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1405,6 +1405,7 @@ VIR_ENUM_DECL(virDomainVideoVGAConf);
 struct _virDomainVideoAccelDef {
 int accel2d; /* enum virTristateBool */
 int accel3d; /* enum virTristateBool */
+char *rendernode;
 };
 
 
-- 
2.21.0

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

[libvirt] [PATCH v2 13/16] qemu: start/stop the vhost-user-gpu external device

[Cole Robinson]

From: Marc-André Lureau 

Each vhost-user-gpu needs its own helper gpu process.
Start/stop them, and apply the emulator cgroup controller.

Signed-off-by: Marc-André Lureau 
Signed-off-by: Cole Robinson 
---
 src/qemu/qemu_extdevice.c | 46 +++
 1 file changed, 42 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
index 5c55aba006..3dbcec3546 100644
--- a/src/qemu/qemu_extdevice.c
+++ b/src/qemu/qemu_extdevice.c
@@ -161,10 +161,21 @@ qemuExtDevicesStart(virQEMUDriverPtr driver,
 bool incomingMigration)
 {
 int ret = 0;
+size_t i;
 
 if (qemuExtDevicesInitPaths(driver, vm->def) < 0)
 return -1;
 
+for (i = 0; i < vm->def->nvideos; i++) {
+virDomainVideoDefPtr video = vm->def->videos[i];
+
+if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && video->vhostuser) {
+ret = qemuExtVhostUserGPUStart(driver, vm, video, logCtxt);
+if (ret < 0)
+return ret;
+}
+}
+
 if (vm->def->tpm)
 ret = qemuExtTPMStart(driver, vm, logCtxt, incomingMigration);
 
@@ -176,9 +187,18 @@ void
 qemuExtDevicesStop(virQEMUDriverPtr driver,
virDomainObjPtr vm)
 {
+size_t i;
+
 if (qemuExtDevicesInitPaths(driver, vm->def) < 0)
 return;
 
+for (i = 0; i < vm->def->nvideos; i++) {
+virDomainVideoDefPtr video = vm->def->videos[i];
+
+if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && video->vhostuser)
+qemuExtVhostUserGPUStop(driver, vm, video);
+}
+
 if (vm->def->tpm)
 qemuExtTPMStop(driver, vm);
 }
@@ -187,6 +207,14 @@ qemuExtDevicesStop(virQEMUDriverPtr driver,
 bool
 qemuExtDevicesHasDevice(virDomainDefPtr def)
 {
+size_t i;
+
+for (i = 0; i < def->nvideos; i++) {
+if (def->videos[i]->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO &&
+def->videos[i]->vhostuser)
+return true;
+}
+
 if (def->tpm && def->tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR)
 return true;
 
@@ -199,10 +227,20 @@ qemuExtDevicesSetupCgroup(virQEMUDriverPtr driver,
   virDomainDefPtr def,
   virCgroupPtr cgroup)
 {
-int ret = 0;
+size_t i;
 
-if (def->tpm)
-ret = qemuExtTPMSetupCgroup(driver, def, cgroup);
+for (i = 0; i < def->nvideos; i++) {
+virDomainVideoDefPtr video = def->videos[i];
 
-return ret;
+if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO &&
+video->vhostuser &&
+qemuExtVhostUserGPUSetupCgroup(driver, def, video, cgroup) < 0)
+return -1;
+}
+
+if (def->tpm &&
+qemuExtTPMSetupCgroup(driver, def, cgroup) < 0)
+return -1;
+
+return 0;
 }
-- 
2.21.0

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

[libvirt] [PATCH v2 15/16] tests: mock execv/execve

[Cole Robinson]

From: Marc-André Lureau 

Learn to override the paths to the program to execute (vhost-user
helpers are executed to check for runtime capabilities).

Signed-off-by: Marc-André Lureau 
Signed-off-by: Cole Robinson 
---
 tests/virfilewrapper.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/tests/virfilewrapper.c b/tests/virfilewrapper.c
index 160cd571e0..3d3f319f2c 100644
--- a/tests/virfilewrapper.c
+++ b/tests/virfilewrapper.c
@@ -44,6 +44,8 @@ static FILE *(*real_fopen)(const char *path, const char 
*mode);
 static int (*real_access)(const char *path, int mode);
 static int (*real_mkdir)(const char *path, mode_t mode);
 static DIR *(*real_opendir)(const char *path);
+static int (*real_execv)(const char *path, char *const argv[]);
+static int (*real_execve)(const char *path, char *const argv[], char *const 
envp[]);
 
 static void init_syms(void)
 {
@@ -55,6 +57,8 @@ static void init_syms(void)
 VIR_MOCK_REAL_INIT(mkdir);
 VIR_MOCK_REAL_INIT(open);
 VIR_MOCK_REAL_INIT(opendir);
+VIR_MOCK_REAL_INIT(execv);
+VIR_MOCK_REAL_INIT(execve);
 }
 
 
@@ -191,4 +195,22 @@ DIR *opendir(const char *path)
 return real_opendir(newpath ? newpath : path);
 }
 
+int execv(const char *path, char *const argv[])
+{
+VIR_AUTOFREE(char *) newpath = NULL;
+
+PATH_OVERRIDE(newpath, path);
+
+return real_execv(newpath ? newpath : path, argv);
+}
+
+int execve(const char *path, char *const argv[], char *const envp[])
+{
+VIR_AUTOFREE(char *) newpath = NULL;
+
+PATH_OVERRIDE(newpath, path);
+
+return real_execve(newpath ? newpath : path, argv, envp);
+}
+
 #endif
-- 
2.21.0

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

[libvirt] [PATCH v2 16/16] tests: add vhost-user-gpu xml2argv tests

[Cole Robinson]

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
Signed-off-by: Cole Robinson 
---
 .../vhost-user-gpu-secondary.args | 38 
 .../vhost-user-gpu-secondary.xml  | 44 +++
 tests/qemuxml2argvdata/vhost-user-vga.args| 35 +++
 tests/qemuxml2argvdata/vhost-user-vga.xml | 41 +
 tests/qemuxml2argvtest.c  | 21 +
 5 files changed, 179 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/vhost-user-gpu-secondary.args
 create mode 100644 tests/qemuxml2argvdata/vhost-user-gpu-secondary.xml
 create mode 100644 tests/qemuxml2argvdata/vhost-user-vga.args
 create mode 100644 tests/qemuxml2argvdata/vhost-user-vga.xml

diff --git a/tests/qemuxml2argvdata/vhost-user-gpu-secondary.args 
b/tests/qemuxml2argvdata/vhost-user-gpu-secondary.args
new file mode 100644
index 00..5d41edad6b
--- /dev/null
+++ b/tests/qemuxml2argvdata/vhost-user-gpu-secondary.args
@@ -0,0 +1,38 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i686 \
+-name QEMUGuest1 \
+-S \
+-machine pc,accel=tcg,usb=off,dump-guest-core=off \
+-m 214 \
+-realtime mlock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-object memory-backend-memfd,id=ram-node0,share=yes,size=224395264 \
+-numa node,nodeid=0,cpus=0,memdev=ram-node0 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev 
socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
+-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
+-chardev socket,id=chr-vu-video0,fd=0 \
+-chardev socket,id=chr-vu-video1,fd=0 \
+-device 
vhost-user-vga,id=video0,max_outputs=1,chardev=chr-vu-video0,bus=pci.0,\
+addr=0x2 \
+-device vhost-user-gpu-pci,id=video1,max_outputs=1,chardev=chr-vu-video1,\
+bus=pci.0,addr=0x4 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/vhost-user-gpu-secondary.xml 
b/tests/qemuxml2argvdata/vhost-user-gpu-secondary.xml
new file mode 100644
index 00..2142497d6e
--- /dev/null
+++ b/tests/qemuxml2argvdata/vhost-user-gpu-secondary.xml
@@ -0,0 +1,44 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219136
+  219136
+  
+
+
+  
+  1
+  
+  
+  
+  
+  
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-i686
+
+  
+  
+  
+
+
+
+
+
+
+
+
+  
+
+
+  
+
+  
+
diff --git a/tests/qemuxml2argvdata/vhost-user-vga.args 
b/tests/qemuxml2argvdata/vhost-user-vga.args
new file mode 100644
index 00..ce690e4f21
--- /dev/null
+++ b/tests/qemuxml2argvdata/vhost-user-vga.args
@@ -0,0 +1,35 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-i686 \
+-name QEMUGuest1 \
+-S \
+-machine pc,accel=tcg,usb=off,dump-guest-core=off \
+-m 214 \
+-realtime mlock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-object memory-backend-memfd,id=ram-node0,share=yes,size=224395264 \
+-numa node,nodeid=0,cpus=0,memdev=ram-node0 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev 
socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\
+server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
+-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \
+-chardev socket,id=chr-vu-video0,fd=0 \
+-device 
vhost-user-vga,id=video0,max_outputs=1,chardev=chr-vu-video0,bus=pci.0,\
+addr=0x2 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvdata/vhost-user-vga.xml 
b/tests/qemuxml2argvdata/vhost-user-vga.xml
new file mode 100644
index 00..81b1e7643e
--- /dev/null
+++ b/tests/qemuxml2argvdata/vhost-user-vga.xml
@@ -0,0 +1,41 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219136
+  219136
+  
+
+
+  
+  1
+  
+
+  
+
+  
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-i686
+
+  
+  
+  
+
+
+
+
+
+
+
+
+  
+

[libvirt] [PATCH v2 02/16] domain: add "vhostuser" attribute to virtio video model

[Cole Robinson]

From: Marc-André Lureau 

Accept a new attribute to specify usage of helper process, ex:

  

  

Signed-off-by: Marc-André Lureau 
Signed-off-by: Cole Robinson 
---
 docs/formatdomain.html.in |  6 ++
 docs/schemas/domaincommon.rng | 11 ++-
 src/conf/domain_conf.c| 14 ++
 src/conf/domain_conf.h|  1 +
 4 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index fcb7c59c00..ec650fbe17 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -7039,6 +7039,12 @@ qemu-kvm -net nic,model=? /dev/null
   Attribute vram64 (since 
1.3.3)
   extends secondary bar and makes it addressable as 64bit memory.
 
+
+  For guest type "kvm" and model type "virtio" there are
+  optional attributes. Attribute vhost-user
+  (since 5.5.0) specify that a
+  vhost-user helper process should be associated with the GPU.
+
   
 
   acceleration
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index c48f8c4f56..bac566855d 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3581,7 +3581,6 @@
 vmvga
 xen
 vbox
-virtio
 gop
 none
 bochs
@@ -3607,6 +3606,16 @@
 
   
 
+
+  
+virtio
+  
+  
+
+  
+
+  
+
   
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b7a342bb91..f51575d57d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15358,6 +15358,7 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt,
 xmlNodePtr cur;
 VIR_XPATH_NODE_AUTORESTORE(ctxt);
 VIR_AUTOFREE(char *) type = NULL;
+VIR_AUTOFREE(char *) vhostuser = NULL;
 VIR_AUTOFREE(char *) heads = NULL;
 VIR_AUTOFREE(char *) vram = NULL;
 VIR_AUTOFREE(char *) vram64 = NULL;
@@ -15376,6 +15377,7 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt,
 if (!type && !vram && !ram && !heads &&
 virXMLNodeNameEqual(cur, "model")) {
 type = virXMLPropString(cur, "type");
+vhostuser = virXMLPropString(cur, "vhostuser");
 ram = virXMLPropString(cur, "ram");
 vram = virXMLPropString(cur, "vram");
 vram64 = virXMLPropString(cur, "vram64");
@@ -15408,6 +15410,16 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt,
 def->type = virDomainVideoDefaultType(dom);
 }
 
+if (vhostuser != NULL) {
+if (virStringParseYesNo(vhostuser, >vhostuser) < 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("unknown vhostuser value '%s'"), vhostuser);
+goto cleanup;
+}
+} else {
+def->vhostuser = false;
+}
+
 if (ram) {
 if (def->type != VIR_DOMAIN_VIDEO_TYPE_QXL) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -26486,6 +26498,8 @@ virDomainVideoDefFormat(virBufferPtr buf,
 virBufferAsprintf(buf, " heads='%u'", def->heads);
 if (def->primary)
 virBufferAddLit(buf, " primary='yes'");
+if (def->vhostuser)
+virBufferAddLit(buf, " vhostuser='yes'");
 if (def->accel) {
 virBufferAddLit(buf, ">\n");
 virBufferAdjustIndent(buf, 2);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 33cef5b75c..bc2450f25e 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1424,6 +1424,7 @@ struct _virDomainVideoDef {
 virDomainVideoDriverDefPtr driver;
 virDomainDeviceInfo info;
 virDomainVirtioOptionsPtr virtio;
+bool vhostuser;
 };
 
 /* graphics console modes */
-- 
2.21.0

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

[libvirt] [PATCH v2 12/16] qemu: prepare domain for vhost-user GPU

[Cole Robinson]

From: Marc-André Lureau 

Call qemuExtVhostUserGPUPrepareDomain() to fill the domain with the
location of the vhost-user binary to start.

Signed-off-by: Marc-André Lureau 
Signed-off-by: Cole Robinson 
---
 src/qemu/qemu_extdevice.c | 29 +
 src/qemu/qemu_extdevice.h |  5 +
 src/qemu/qemu_process.c   |  4 
 3 files changed, 38 insertions(+)

diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
index dc032aa60c..5c55aba006 100644
--- a/src/qemu/qemu_extdevice.c
+++ b/src/qemu/qemu_extdevice.c
@@ -21,6 +21,7 @@
 #include 
 
 #include "qemu_extdevice.h"
+#include "qemu_vhost_user_gpu.h"
 #include "qemu_domain.h"
 #include "qemu_tpm.h"
 
@@ -92,6 +93,34 @@ qemuExtDevicesInitPaths(virQEMUDriverPtr driver,
 }
 
 
+/*
+ * qemuExtDevicesPrepareDomain:
+ *
+ * @driver: QEMU driver
+ * @vm: domain
+ *
+ * Code that modifies live XML of a domain which is about to start.
+ */
+int
+qemuExtDevicesPrepareDomain(virQEMUDriverPtr driver,
+virDomainObjPtr vm)
+{
+int ret = 0;
+size_t i;
+
+for (i = 0; i < vm->def->nvideos; i++) {
+virDomainVideoDefPtr video = vm->def->videos[i];
+
+if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO && video->vhostuser) {
+if ((ret = qemuExtVhostUserGPUPrepareDomain(driver, video) < 0))
+break;
+}
+}
+
+return ret;
+}
+
+
 /*
  * qemuExtDevicesPrepareHost:
  *
diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h
index 039b3e60dd..2412244d60 100644
--- a/src/qemu/qemu_extdevice.h
+++ b/src/qemu/qemu_extdevice.h
@@ -29,6 +29,11 @@ int qemuExtDeviceLogCommand(qemuDomainLogContextPtr logCtxt,
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
 ATTRIBUTE_RETURN_CHECK;
 
+int qemuExtDevicesPrepareDomain(virQEMUDriverPtr driver,
+virDomainObjPtr vm)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
+ATTRIBUTE_RETURN_CHECK;
+
 int qemuExtDevicesPrepareHost(virQEMUDriverPtr driver,
   virDomainDefPtr def)
 ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index c439f17011..ed767b0807 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6358,6 +6358,10 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver,
 if (qemuFirmwareFillDomain(driver, vm, flags) < 0)
 goto cleanup;
 
+VIR_DEBUG("Preparing external devices");
+if (qemuExtDevicesPrepareDomain(driver, vm) < 0)
+goto cleanup;
+
 for (i = 0; i < vm->def->nchannels; i++) {
 if (qemuDomainPrepareChannel(vm->def->channels[i],
  priv->channelTargetDir) < 0)
-- 
2.21.0

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

[libvirt] [PATCH v2 10/16] qemu: add qemuSecurityStartVhostUserGPU helper

[Cole Robinson]

From: Marc-André Lureau 

See function documentation. Used in a following patch.

Signed-off-by: Marc-André Lureau 
Signed-off-by: Cole Robinson 
---
 src/qemu/qemu_security.c | 47 
 src/qemu/qemu_security.h |  6 +
 2 files changed, 53 insertions(+)

diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c
index 3cd6d9bd3d..86b06594f6 100644
--- a/src/qemu/qemu_security.c
+++ b/src/qemu/qemu_security.c
@@ -433,6 +433,53 @@ qemuSecurityRestoreChardevLabel(virQEMUDriverPtr driver,
 }
 
 
+/*
+ * qemuSecurityStartVhostUserGPU:
+ *
+ * @driver: the QEMU driver
+ * @vm: the domain object
+ * @cmd: the command to run
+ * @existstatus: pointer to int returning exit status of process
+ * @cmdret: pointer to int returning result of virCommandRun
+ *
+ * Start the vhost-user-gpu process with approriate labels.
+ * This function returns -1 on security setup error, 0 if all the
+ * setup was done properly. In case the virCommand failed to run
+ * 0 is returned but cmdret is set appropriately with the process
+ * exitstatus also set.
+ */
+int
+qemuSecurityStartVhostUserGPU(virQEMUDriverPtr driver,
+  virDomainObjPtr vm,
+  virCommandPtr cmd,
+  int *exitstatus,
+  int *cmdret)
+{
+int ret = -1;
+
+if (virSecurityManagerSetChildProcessLabel(driver->securityManager,
+   vm->def, cmd) < 0)
+goto cleanup;
+
+if (virSecurityManagerPreFork(driver->securityManager) < 0)
+goto cleanup;
+
+ret = 0;
+
+*cmdret = virCommandRun(cmd, exitstatus);
+
+virSecurityManagerPostFork(driver->securityManager);
+
+if (*cmdret < 0)
+goto cleanup;
+
+return 0;
+
+ cleanup:
+return ret;
+}
+
+
 /*
  * qemuSecurityStartTPMEmulator:
  *
diff --git a/src/qemu/qemu_security.h b/src/qemu/qemu_security.h
index 68e377f418..a48ed8ec78 100644
--- a/src/qemu/qemu_security.h
+++ b/src/qemu/qemu_security.h
@@ -77,6 +77,12 @@ int qemuSecurityRestoreChardevLabel(virQEMUDriverPtr driver,
 virDomainObjPtr vm,
 virDomainChrDefPtr chr);
 
+int qemuSecurityStartVhostUserGPU(virQEMUDriverPtr driver,
+  virDomainObjPtr vm,
+  virCommandPtr cmd,
+  int *exitstatus,
+  int *cmdret);
+
 int qemuSecurityStartTPMEmulator(virQEMUDriverPtr driver,
  virDomainObjPtr vm,
  virCommandPtr cmd,
-- 
2.21.0

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

[libvirt] [PATCH v2 00/16] Add vhost-user-gpu support

[Cole Robinson]

v1: https://www.redhat.com/archives/libvir-list/2019-June/msg00102.html

This is v2 of Marc-André's series with minor changes. I'm not taking over
this series, I just fixed these as part of the patch rebase so I can review
it :)

Changes since v1:
- rebase to master
- if test file build by dropping LDADDS usage
- syntax-check issues:
* use #pragma once
* if () bracket issues
* jump label indent issues
* error message %s usage
* size_t for loops

I didn't know much about vhost-user-gpu before this series, here's what
I've learned.

vhost-user is a generic mechanism that allows implementing virtio device
dataplane handling in a separate userspace process. vhost-user-gpu here
notably moves the virgl 3d handling out of the main qemu process. The
external process will be /usr/libexec/vhost-user-gpu, which comes from
qemu.git contrib/vhost-user-gpu code, first released in qemu-4.1.

Part of this series deals with discovering the location on disk of the
vhost-user-gpu binary, and what capabilities it provides. This uses a
similar mechanism to firmware.json, described in qemu
docs/interop/vhost-user.json

https://github.com/qemu/qemu/blob/master/docs/interop/vhost-user.json

qemu 4.1 ships a 50-qemu-gpu.json to match. I believe virtio-fs
will use a similar mechanism when it lands in upstream qemu, as
virtiofsd is a separate process that communicates with qemu over
vhost-user.

For a bit more background on vhost-user-gpu process handling and
the json interop motivation, here's some of the qemu discussion:

https://lists.nongnu.org/archive/html/qemu-devel/2018-08/msg02610.html
https://lists.nongnu.org/archive/html/qemu-devel/2018-09/msg00807.html


For this series, the XML to enable this is:

  

  

rendernode is optional
qemu_vhost_user.c handles vhost-user.json
qemu_vhost_user_gpu.c handles the process management for
vhost-user-gpu


Marc-André Lureau (16):
  qemu: extract out qemuFetchConfigs from firmware
  domain: add "vhostuser" attribute to virtio video model
  domain: add rendernode attribute on 
  qemu-cgroup: allow accel rendernode access
  qemu: add vhost-user-gpu capabilities checks
  qemu: check that qemu is vhost-user-vga capable
  qemu: validate virtio-gpu with vhost-user
  qemu: restrict 'virgl=' option to non-vhostuser video type
  qemu: add vhost-user helpers
  qemu: add qemuSecurityStartVhostUserGPU helper
  qemu: add vhost-user-gpu helper unit
  qemu: prepare domain for vhost-user GPU
  qemu: start/stop the vhost-user-gpu external device
  qemu: build vhost-user GPU devices
  tests: mock execv/execve
  tests: add vhost-user-gpu xml2argv tests

 docs/formatdomain.html.in |  11 +
 docs/schemas/domaincommon.rng |  16 +-
 src/conf/device_conf.c|   1 +
 src/conf/device_conf.h|   2 +
 src/conf/domain_conf.c|  32 +-
 src/conf/domain_conf.h|   2 +
 src/qemu/Makefile.inc.am  |   6 +
 src/qemu/qemu_capabilities.c  |   4 +
 src/qemu/qemu_capabilities.h  |   2 +
 src/qemu/qemu_cgroup.c|  24 ++
 src/qemu/qemu_command.c   |  54 ++-
 src/qemu/qemu_configs.c   | 183 
 src/qemu/qemu_configs.h   |  28 ++
 src/qemu/qemu_domain.c|  11 +-
 src/qemu/qemu_extdevice.c |  75 +++-
 src/qemu/qemu_extdevice.h |   5 +
 src/qemu/qemu_firmware.c  | 144 +--
 src/qemu/qemu_process.c   |  18 +-
 src/qemu/qemu_security.c  |  47 +++
 src/qemu/qemu_security.h  |   6 +
 src/qemu/qemu_vhost_user.c| 394 ++
 src/qemu/qemu_vhost_user.h|  48 +++
 src/qemu/qemu_vhost_user_gpu.c| 305 ++
 src/qemu/qemu_vhost_user_gpu.h|  50 +++
 tests/Makefile.am |   9 +
 .../caps_4.1.0.x86_64.xml |   2 +
 .../etc/qemu/vhost-user/40-gpu.json   |   1 +
 .../etc/qemu/vhost-user/50-gpu.json   |   0
 .../qemu/vhost-user/test-vhost-user-gpu   |  11 +
 .../usr/share/qemu/vhost-user/30-gpu.json |   1 +
 .../usr/share/qemu/vhost-user/50-gpu.json |   8 +
 .../usr/share/qemu/vhost-user/60-gpu.json |   1 +
 tests/qemuvhostusertest.c | 132 ++
 .../vhost-user-gpu-secondary.args |  38 ++
 .../vhost-user-gpu-secondary.xml  |  44 ++
 tests/qemuxml2argvdata/vhost-user-vga.args|  35 ++
 tests/qemuxml2argvdata/vhost-user-vga.xml |  41 ++
 tests/qemuxml2argvtest.c  |  21 +
 tests/virfilewrapper.c|  22 +
 39 files changed, 1676 insertions(+), 158 deletions(-)
 create mode 100644 src/qemu/qemu_configs.c
 create mode 100644 

[libvirt] [PATCH v2 01/16] qemu: extract out qemuFetchConfigs from firmware

[Cole Robinson]

From: Marc-André Lureau 

The same config files disovery & priority rules are used for
vhost-user backends.

Signed-off-by: Marc-André Lureau 
Signed-off-by: Cole Robinson 
---
 src/qemu/Makefile.inc.am |   2 +
 src/qemu/qemu_configs.c  | 183 +++
 src/qemu/qemu_configs.h  |  28 ++
 src/qemu/qemu_firmware.c | 144 +-
 4 files changed, 215 insertions(+), 142 deletions(-)
 create mode 100644 src/qemu/qemu_configs.c
 create mode 100644 src/qemu/qemu_configs.h

diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
index 30a9751cfd..f7a0fa4a84 100644
--- a/src/qemu/Makefile.inc.am
+++ b/src/qemu/Makefile.inc.am
@@ -30,6 +30,8 @@ QEMU_DRIVER_SOURCES = \
qemu/qemu_hotplugpriv.h \
qemu/qemu_conf.c \
qemu/qemu_conf.h \
+   qemu/qemu_configs.c \
+   qemu/qemu_configs.h \
qemu/qemu_process.c \
qemu/qemu_process.h \
qemu/qemu_processpriv.h \
diff --git a/src/qemu/qemu_configs.c b/src/qemu/qemu_configs.c
new file mode 100644
index 00..39b8906be5
--- /dev/null
+++ b/src/qemu/qemu_configs.c
@@ -0,0 +1,183 @@
+/*
+ * qemu_configs.c: QEMU firmware/vhost-user etc configs
+ *
+ * Copyright (C) 2019 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
+ * .
+ */
+
+#include 
+
+#include "qemu_configs.h"
+#include "configmake.h"
+#include "viralloc.h"
+#include "virenum.h"
+#include "virfile.h"
+#include "virhash.h"
+#include "virlog.h"
+#include "virstring.h"
+
+#define VIR_FROM_THIS VIR_FROM_QEMU
+
+VIR_LOG_INIT("qemu.qemu_configs");
+
+static int
+qemuBuildFileList(virHashTablePtr files, const char *dir)
+{
+DIR *dirp;
+struct dirent *ent = NULL;
+int rc;
+int ret = -1;
+
+if ((rc = virDirOpenIfExists(, dir)) < 0)
+return -1;
+
+if (rc == 0)
+return 0;
+
+while ((rc = virDirRead(dirp, , dir)) > 0) {
+VIR_AUTOFREE(char *) filename = NULL;
+VIR_AUTOFREE(char *) path = NULL;
+struct stat sb;
+
+if (STRPREFIX(ent->d_name, "."))
+continue;
+
+if (VIR_STRDUP(filename, ent->d_name) < 0)
+goto cleanup;
+
+if (virAsprintf(, "%s/%s", dir, filename) < 0)
+goto cleanup;
+
+if (stat(path, ) < 0) {
+virReportSystemError(errno, _("Unable to access %s"), path);
+goto cleanup;
+}
+
+if (!S_ISREG(sb.st_mode) && !S_ISLNK(sb.st_mode))
+continue;
+
+if (virHashUpdateEntry(files, filename, path) < 0)
+goto cleanup;
+
+path = NULL;
+}
+
+if (rc < 0)
+goto cleanup;
+
+ret = 0;
+ cleanup:
+virDirClose();
+return ret;
+}
+
+static int
+qemuConfigFilesSorter(const virHashKeyValuePair *a,
+const virHashKeyValuePair *b)
+{
+return strcmp(a->key, b->key);
+}
+
+#define QEMU_SYSTEM_LOCATION PREFIX "/share/qemu"
+#define QEMU_ETC_LOCATION SYSCONFDIR "/qemu"
+
+int
+qemuFetchConfigs(const char *name,
+ char ***configs,
+ bool privileged)
+{
+VIR_AUTOPTR(virHashTable) files = NULL;
+VIR_AUTOFREE(char *) homeConfig = NULL;
+VIR_AUTOFREE(char *) xdgConfig = NULL;
+VIR_AUTOFREE(char *) sysLocation = virFileBuildPath(QEMU_SYSTEM_LOCATION, 
name, NULL);
+VIR_AUTOFREE(char *) etcLocation = virFileBuildPath(QEMU_ETC_LOCATION, 
name, NULL);
+VIR_AUTOFREE(virHashKeyValuePairPtr) pairs = NULL;
+virHashKeyValuePairPtr tmp = NULL;
+
+*configs = NULL;
+
+if (!privileged) {
+/* This is a slight divergence from the specification.
+ * Since the system daemon runs as root, it doesn't make
+ * much sense to parse files in root's home directory. It
+ * makes sense only for session daemon which runs under
+ * regular user. */
+if (VIR_STRDUP(xdgConfig, getenv("XDG_CONFIG_HOME")) < 0)
+return -1;
+
+if (!xdgConfig) {
+VIR_AUTOFREE(char *) home = virGetUserDirectory();
+
+if (!home)
+return -1;
+
+if (virAsprintf(, "%s/.config", home) < 0)
+return -1;
+}
+
+if (virAsprintf(, "%s/qemu/%s", xdgConfig, name) < 0)
+return -1;
+}
+
+if (!(files = virHashCreate(10, virHashValueFree)))
+return -1;
+
+

[libvirt] [PATCH 2/4] remote: move timeout arg into sysconf file

[Daniel P . Berrangé]

We need to give users the ability to customize the length of the
shutdown timeout, or even disable timeouts entirely. Thus we must move
the timeout arg into the sysconf file, instead of the service unit.

Signed-off-by: Daniel P. Berrangé 
---
 src/remote/libvirtd.service.in |  6 +-
 src/remote/libvirtd.sysconf| 12 +---
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in
index 82892b4f70..9c8c54a2ef 100644
--- a/src/remote/libvirtd.service.in
+++ b/src/remote/libvirtd.service.in
@@ -26,11 +26,7 @@ Documentation=https://libvirt.org
 [Service]
 Type=notify
 EnvironmentFile=-@sysconfdir@/sysconfig/libvirtd
-# libvirtd.service is set to run on boot so that autostart of
-# VMs can be performed. We don't want it to stick around if
-# unused though, so we set a timeout. The socket activation
-# then ensures it gets started again if anything needs it
-ExecStart=@sbindir@/libvirtd --timeout 120 $LIBVIRTD_ARGS
+ExecStart=@sbindir@/libvirtd $LIBVIRTD_ARGS
 ExecReload=/bin/kill -HUP $MAINPID
 KillMode=process
 Restart=on-failure
diff --git a/src/remote/libvirtd.sysconf b/src/remote/libvirtd.sysconf
index 5969518bf2..2ad1fcf5d5 100644
--- a/src/remote/libvirtd.sysconf
+++ b/src/remote/libvirtd.sysconf
@@ -1,8 +1,14 @@
 # Customizations for the libvirtd.service systemd unit
 
-# Listen for TCP/IP connections. This is not required if using systemd
-# socket activation.
-# NB. must setup TLS/SSL keys prior to using this
+# Default behaviour is for libvirtd.service to start on boot
+# so that VM autostart can be performed. We then want it to
+# shutdown again if nothing was started and rely on systemd
+# socket activation to start it again when some client app
+# connects.
+LIBVIRT_ARGS="--timeout 120"
+
+# If systemd socket activation is disabled, then the following
+# can be used to listen on TCP/TLS sockets
 #LIBVIRTD_ARGS="--listen"
 
 # Override Kerberos service keytab for SASL/GSSAPI
-- 
2.21.0

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

[libvirt] [PATCH 3/4] remote: forbid the --listen arg when systemd socket activation

[Daniel P . Berrangé]

When using systemd socket activation the --listen arg has no
effect. This is confusing to users upgrading from previous versions of
libvirt as their config is silently ignored. Turn use of --listen into a
fatal error when sockets are passed from systemd.

This helps the admin discover the change in behaviour and thus decide
whether to stick with socket activation or revert to previous behaviour.

Signed-off-by: Daniel P. Berrangé 
---
 src/remote/libvirtd.pod| 33 -
 src/remote/remote_daemon.c |  7 +++
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/src/remote/libvirtd.pod b/src/remote/libvirtd.pod
index 4721e0f4ec..fa30d6a37a 100644
--- a/src/remote/libvirtd.pod
+++ b/src/remote/libvirtd.pod
@@ -30,6 +30,35 @@ and will be picked up automatically if their XML 
configuration has been
 defined.  Any guests whose XML configuration has not been defined will be lost
 from the configuration.
 
+=head1 SYSTEM SOCKET ACTIVATION
+
+The B daemon is capable of starting in two modes.
+
+In the traditional mode, it will create and listen on UNIX sockets itself.
+If the B<--listen> parameter is given, it will also listen on TCP/IP socket(s),
+according to the B and B options in
+B
+
+In socket activation mode, it will rely on systemd to create and listen
+on the UNIX, and optionally TCP/IP, sockets and pass them as pre-opened
+file descriptors. In this mode, it is not permitted to pass the B<--listen>
+parameter, and most of the socket related config options in
+B will no longer have any effect. To enable
+TCP or TLS sockets use either
+
+B<$ systemctl start libvirtd-tls.socket>
+
+Or
+
+B<$ systemctl start libvirtd-tcp.socket>
+
+Socket activation mode is generally the default when running on a host
+OS that uses systemd. To revert to the traditional mode, all the socket
+unit files must be masked:
+
+B<$ systemctl mask libvirtd.socket libvirtd-ro.socket \
+  libvirtd-admin.socket libvirtd-tls.socket libvirtd-tcp.socket>
+
 =head1 OPTIONS
 
 =over
@@ -48,7 +77,9 @@ Use this configuration file, overriding the default value.
 
 =item B<-l, --listen>
 
-Listen for TCP/IP connections.
+Listen for TCP/IP connections. This should not be set if using systemd
+socket activation. Instead activate the libvirtd-tls.socket or
+libvirtd-tcp.socket unit files.
 
 =item B<-p, --pid-file> I
 
diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
index 1138485870..3970db09c0 100644
--- a/src/remote/remote_daemon.c
+++ b/src/remote/remote_daemon.c
@@ -422,6 +422,13 @@ daemonSetupNetworking(virNetServerPtr srv,
 if (virSystemdGetActivation(actmap, ARRAY_CARDINALITY(actmap), ) < 0)
 return -1;
 
+#ifdef WITH_IP
+if (act && ipsock) {
+VIR_ERROR(_("--listen parameter not permitted with systemd activation 
sockets"));
+return -1;
+}
+#endif /* ! WITH_IP */
+
 if (config->unix_sock_group) {
 if (virGetGroupID(config->unix_sock_group, _sock_gid) < 0)
 return ret;
-- 
2.21.0

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

[libvirt] [PATCH 4/4] rpm: don't enable socket activation in upgrade if --listen present

[Daniel P . Berrangé]

Currently during RPM upgrade we restart libvirtd and unconditionally
enable use of systemd socket activation for the UNIX sockets.

If the user had previously given the --listen arg to libvirtd though,
this will no longer be honoured if socket activation is used.

We could start libvirtd-tcp.socket or libvirtd-tls.socket for this,
but mgmt tools like puppet/ansible might not be expecting this.
So for now we silently disable socket activation if we see --listen
was previously set on the host.

Signed-off-by: Daniel P. Berrangé 
---
 libvirt.spec.in | 44 +++-
 1 file changed, 31 insertions(+), 13 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index ee4b408510..e6c85a706b 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1379,19 +1379,37 @@ fi
 
 %posttrans daemon
 if [ -f %{_localstatedir}/lib/rpm-state/libvirt/restart ]; then
-# Old libvirtd owns the sockets and will delete them on
-# shutdown. Can't use a try-restart as libvirtd will simply
-# own the sockets again when it comes back up. Thus we must
-# do this particular ordering
-/bin/systemctl is-active libvirtd.service 1>/dev/null 2>&1
-if test $? = 0 ; then
-/bin/systemctl stop libvirtd.service >/dev/null 2>&1 || :
-
-/bin/systemctl try-restart libvirtd.socket >/dev/null 2>&1 || :
-/bin/systemctl try-restart libvirtd-ro.socket >/dev/null 2>&1 || :
-/bin/systemctl try-restart libvirtd-admin.socket >/dev/null 2>&1 || :
-
-/bin/systemctl start libvirtd.service >/dev/null 2>&1 || :
+# See if user has previously modified their install to
+# tell libvirtd to use --listen
+grep -E '^LIBVIRTD_ARGS=.*--listen' /etc/sysconfig/libvirtd 1>/dev/null 
2>&1
+if test $? = 0
+then
+# Then lets keep honouring --listen and *not* use
+# systemd socket activation, because switching things
+# might confuse mgmt tool like puppet/ansible that
+# expect the old style libvirtd
+/bin/systemctl mask libvirtd.socket >/dev/null 2>&1 || :
+/bin/systemctl mask libvirtd-ro.socket >/dev/null 2>&1 || :
+/bin/systemctl mask libvirtd-admin.socket >/dev/null 2>&1 || :
+/bin/systemctl mask libvirtd-tls.socket >/dev/null 2>&1 || :
+/bin/systemctl mask libvirtd-tcp.socket >/dev/null 2>&1 || :
+else
+# Old libvirtd owns the sockets and will delete them on
+# shutdown. Can't use a try-restart as libvirtd will simply
+# own the sockets again when it comes back up. Thus we must
+# do this particular ordering, so that we get libvirtd
+# running with socket activation in use
+/bin/systemctl is-active libvirtd.service 1>/dev/null 2>&1
+if test $? = 0
+then
+/bin/systemctl stop libvirtd.service >/dev/null 2>&1 || :
+
+/bin/systemctl try-restart libvirtd.socket >/dev/null 2>&1 || :
+/bin/systemctl try-restart libvirtd-ro.socket >/dev/null 2>&1 || :
+/bin/systemctl try-restart libvirtd-admin.socket >/dev/null 2>&1 
|| :
+
+/bin/systemctl start libvirtd.service >/dev/null 2>&1 || :
+fi
 fi
 fi
 rm -rf %{_localstatedir}/lib/rpm-state/libvirt || :
-- 
2.21.0

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

[libvirt] [PATCH 0/4] remote: better handle system activation and upgrades

[Daniel P . Berrangé]

This improves the upgrade path to systemd socket activation

 - Disable socket activation in RPM %post if we see use of --listen
   arg on existing install
 - Report fatal error if --listen is used with socket activation,
   since it is not honoured

Daniel P. Berrangé (4):
  remote: use Wants instead of Requires for libvirtd sockets
  remote: move timeout arg into sysconf file
  remote: forbid the --listen arg when systemd socket activation
  rpm: don't enable socket activation in upgrade if --listen present

 libvirt.spec.in| 44 --
 src/remote/libvirtd.pod| 33 -
 src/remote/libvirtd.service.in | 15 ++--
 src/remote/libvirtd.sysconf| 12 +++---
 src/remote/remote_daemon.c |  7 ++
 5 files changed, 86 insertions(+), 25 deletions(-)

-- 
2.21.0

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

[libvirt] [PATCH 1/4] remote: use Wants instead of Requires for libvirtd sockets

[Daniel P . Berrangé]

To facilitate upgrades from earlier versions of libvirt which did not
use socket activation for libvirtd, we want to allow the libvirtd socket
units to be disabled (masked). This can only be supported if we use the
warker Wants statement instead of Requires.

Signed-off-by: Daniel P. Berrangé 
---
 src/remote/libvirtd.service.in | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in
index 4c5b28b478..82892b4f70 100644
--- a/src/remote/libvirtd.service.in
+++ b/src/remote/libvirtd.service.in
@@ -2,9 +2,12 @@
 Description=Virtualization daemon
 Requires=virtlogd.socket
 Requires=virtlockd.socket
-Requires=libvirtd.socket
-Requires=libvirtd-ro.socket
-Requires=libvirtd-admin.socket
+# Use Wants instead of Requires so that users
+# can disable these three .socket units to revert
+# to a traditional non-activation deployment setup
+Wants=libvirtd.socket
+Wants=libvirtd-ro.socket
+Wants=libvirtd-admin.socket
 Wants=systemd-machined.service
 Before=libvirt-guests.service
 After=network.target
-- 
2.21.0

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

Re: [libvirt] [PATCH v2 7/9] qemu: add helper for getting full FSInfo

[Jonathon Jongsma]

On Thu, 2019-08-22 at 19:02 -0300, Daniel Henrique Barboza wrote:
> This patch fails to compile in my env throwing this error:
> 
> 
>   CC   qemuagenttest.o
> qemuagenttest.c: In function
> 'testQemuAgentGetFSInfoCommon.constprop':
> qemuagenttest.c:242:5: error: 'ret_def' may be used uninitialized in
> this function [-Werror=maybe-uninitialized]
>   242 | virDomainDefFree(ret_def);
>   | ^
> cc1: all warnings being treated as errors
> make[2]: *** [Makefile:5856: qemuagenttest.o] Error 1
> 
> 
> I attempted to fix it by doing:
> 
> 
> 
> diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c
> index be53f7e61a..00dcf26681 100644
> --- a/tests/qemuagenttest.c
> +++ b/tests/qemuagenttest.c
> @@ -175,7 +175,7 @@
> testQemuAgentGetFSInfoCommon(virDomainXMLOptionPtr xmlopt,
>  int ret = -1;
>  char *domain_filename = NULL;
>  qemuMonitorTestPtr ret_test;
> -virDomainDefPtr ret_def;
> +virDomainDefPtr ret_def = NULL;
>  
>  if (!test || !def)
>  return -1;
> 
> --
> 
> I was able to compile it, but then 'make check' got rekt:
> 
> 
> [...]
> PASS: qemudomainsnapshotxml2xmltest
> PASS: qemucommandutiltest
> PASS: virsh-cpuset
> ../build-aux/test-driver: line 107: 19017 Segmentation fault 
> (core dumped) "$@" > $log_file 2>&1
> FAIL: qemuagenttest
> PASS: qemumigparamstest
> [...]
> =
> ===
> Testsuite summary for libvirt 5.7.0
> =
> ===
> # TOTAL: 129
> # PASS:  127
> # SKIP:  1
> # XFAIL: 0
> # FAIL:  1
> # XPASS: 0
> # ERROR: 0
> -
> 
> 
> Not sure if there is something wrong with the patch, or with the fix
> I made (seems unlikely, but ...) or perhaps something else missing in
> my
> env (is there any external libs needed for this?).
> 
> 
> Thanks,
> 
> 
> DHB

Hmm, sorry for the sloppiness. I ran these tests multiple times, but
apparently after a final rebase, I introduced something that I failed
to catch before submitting. Thanks for testing it out.

Jonathon


> 
> 
> On 8/21/19 7:15 PM, Jonathon Jongsma wrote:
> > This function adds the complete filesystem information returned by
> > the
> > qemu agent to an array of typed parameters with field names
> > intended to
> > to be returned by virDomainGetGuestInfo()
> > ---
> >  src/qemu/qemu_agent.c |  89 ++
> >  src/qemu/qemu_agent.h |   5 +
> >  tests/qemuagenttest.c | 210
> > +++---
> >  3 files changed, 291 insertions(+), 13 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
> > index d5519cb243..c101805b23 100644
> > --- a/src/qemu/qemu_agent.c
> > +++ b/src/qemu/qemu_agent.c
> > @@ -1953,6 +1953,95 @@ qemuAgentGetFSInfo(qemuAgentPtr mon,
> > virDomainFSInfoPtr **info,
> >  return ret;
> >  }
> >  
> > +int
> > +qemuAgentGetFSInfoParams(qemuAgentPtr mon,
> > + virTypedParameterPtr *params,
> > + int *nparams, int *maxparams,
> > + virDomainDefPtr vmdef)
> > +{
> > +int ret = -1;
> > +qemuAgentFSInfoPtr *fsinfo = NULL;
> > +size_t i, j;
> > +int nfs;
> > +
> > +nfs = qemuAgentGetFSInfoInternal(mon, , vmdef);
> > +
> > +if (virTypedParamsAddUInt(params, nparams, maxparams,
> > +  "fs.count", nfs) < 0)
> > +goto cleanup;
> > +
> > +for (i = 0; i < nfs; i++) {
> > +char param_name[VIR_TYPED_PARAM_FIELD_LENGTH];
> > +snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> > + "fs.%zu.name", i);
> > +if (virTypedParamsAddString(params, nparams, maxparams,
> > +param_name, fsinfo[i]->name) <
> > 0)
> > +goto cleanup;
> > +snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> > + "fs.%zu.mountpoint", i);
> > +if (virTypedParamsAddString(params, nparams, maxparams,
> > +param_name, fsinfo[i]-
> > >mountpoint) < 0)
> > +goto cleanup;
> > +snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> > + "fs.%zu.fstype", i);
> > +if (virTypedParamsAddString(params, nparams, maxparams,
> > +param_name, fsinfo[i]->fstype) 
> > < 0)
> > +goto cleanup;
> > +
> > +/* disk usage values are not returned by older guest
> > agents, so
> > + * only add the params if the value is set */
> > +snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH,
> > + "fs.%zu.total-bytes", i);
> > +if (fsinfo[i]->total_bytes != -1 &&
> > +virTypedParamsAddULLong(params, nparams, maxparams,
> > +param_name, fsinfo[i]-
> > >total_bytes) < 0)
> > +goto cleanup;
> > +
> > +snprintf(param_name, 

Re: [libvirt] [PATCH 00/19] Add vhost-user-gpu support

[Ján Tomko]

On Thu, Aug 22, 2019 at 07:50:14PM -0400, Cole Robinson wrote:

On 8/1/19 5:28 AM, Marc-André Lureau wrote:

Hi

On Wed, Jun 5, 2019 at 2:32 PM  wrote:


From: Marc-André Lureau 

Hi,

This series of patches adds support for running virtio GPUs in
seperate processes, thanks to vhost-user backend.

The QEMU support landed for 4.1. There are several benefits of running
the GPU/virgl in an external process, since Mesa is rather heavy on
the qemu main loop, and may block for a while, or crash.

The external GPU process is started with one end of a socket pair, the
other end is given to a QEMU chardev attached to a device. The
external process is also added to the cgroup to limit resources usage.

Thanks

Since RFC:
- discover helpers following the vhost-user spec
- change vhost-user  model for a vhostuser attribute
- add a patch to specify the rendernode on 
- change the way command line is built, following qemu series changes
- socket labeling
- a few cleanup patches
- rebased


Can I get some reviews before I respin the series?


I rebased the series and fixed all the 'make syntax-check' errors,
patches are here:

https://github.com/crobinso/libvirt/tree/vhost-user-gpu

I'll do a review tomorrow



If you plan to review your modified version, please send it to the list
first

Jano


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

[libvirt] [PATCH] virsh: Allow graceful console shutdown

[Michal Privoznik]

Currently, whenever there's a regular EOF on the console stream
or an error the virStreamAbort() is called regardless. While this
may not actually break anything, we should call virStreamFinish()
to let the daemon know we've successfully received all the data
and are shutting down the stream gracefully.

Signed-off-by: Michal Privoznik 
---
 tools/virsh-console.c | 41 +
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/tools/virsh-console.c b/tools/virsh-console.c
index a235a9a283..900faa5087 100644
--- a/tools/virsh-console.c
+++ b/tools/virsh-console.c
@@ -97,7 +97,8 @@ virConsoleHandleSignal(int sig ATTRIBUTE_UNUSED)
 
 
 static void
-virConsoleShutdown(virConsolePtr con)
+virConsoleShutdown(virConsolePtr con,
+   bool graceful)
 {
 virErrorPtr err = virGetLastError();
 
@@ -105,10 +106,18 @@ virConsoleShutdown(virConsolePtr con)
 virCopyLastError(>error);
 
 if (con->st) {
+int rc;
+
 virStreamEventRemoveCallback(con->st);
-if (virStreamAbort(con->st) < 0)
+if (graceful)
+rc = virStreamFinish(con->st);
+else
+rc = virStreamAbort(con->st);
+
+if (rc < 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("cannot terminate console stream"));
+}
 virStreamFree(con->st);
 con->st = NULL;
 }
@@ -160,7 +169,7 @@ virConsoleEventOnStream(virStreamPtr st,
 if (avail < 1024) {
 if (VIR_REALLOC_N(con->streamToTerminal.data,
   con->streamToTerminal.length + 1024) < 0) {
-virConsoleShutdown(con);
+virConsoleShutdown(con, false);
 goto cleanup;
 }
 con->streamToTerminal.length += 1024;
@@ -174,7 +183,7 @@ virConsoleEventOnStream(virStreamPtr st,
 if (got == -2)
 goto cleanup; /* blocking */
 if (got <= 0) {
-virConsoleShutdown(con);
+virConsoleShutdown(con, got == 0);
 goto cleanup;
 }
 con->streamToTerminal.offset += got;
@@ -193,7 +202,7 @@ virConsoleEventOnStream(virStreamPtr st,
 if (done == -2)
 goto cleanup; /* blocking */
 if (done < 0) {
-virConsoleShutdown(con);
+virConsoleShutdown(con, done == 0);
 goto cleanup;
 }
 memmove(con->terminalToStream.data,
@@ -214,7 +223,7 @@ virConsoleEventOnStream(virStreamPtr st,
 
 if (events & VIR_STREAM_EVENT_ERROR ||
 events & VIR_STREAM_EVENT_HANGUP) {
-virConsoleShutdown(con);
+virConsoleShutdown(con, false);
 }
 
  cleanup:
@@ -244,7 +253,7 @@ virConsoleEventOnStdin(int watch ATTRIBUTE_UNUSED,
 if (avail < 1024) {
 if (VIR_REALLOC_N(con->terminalToStream.data,
   con->terminalToStream.length + 1024) < 0) {
-virConsoleShutdown(con);
+virConsoleShutdown(con, false);
 goto cleanup;
 }
 con->terminalToStream.length += 1024;
@@ -258,17 +267,17 @@ virConsoleEventOnStdin(int watch ATTRIBUTE_UNUSED,
 if (got < 0) {
 if (errno != EAGAIN) {
 virReportSystemError(errno, "%s", _("cannot read from stdin"));
-virConsoleShutdown(con);
+virConsoleShutdown(con, false);
 }
 goto cleanup;
 }
 if (got == 0) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("EOF on stdin"));
-virConsoleShutdown(con);
+virConsoleShutdown(con, false);
 goto cleanup;
 }
 if (con->terminalToStream.data[con->terminalToStream.offset] == 
con->escapeChar) {
-virConsoleShutdown(con);
+virConsoleShutdown(con, true);
 goto cleanup;
 }
 
@@ -281,13 +290,13 @@ virConsoleEventOnStdin(int watch ATTRIBUTE_UNUSED,
 
 if (events & VIR_EVENT_HANDLE_ERROR) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("IO error on stdin"));
-virConsoleShutdown(con);
+virConsoleShutdown(con, false);
 goto cleanup;
 }
 
 if (events & VIR_EVENT_HANDLE_HANGUP) {
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("EOF on stdin"));
-virConsoleShutdown(con);
+virConsoleShutdown(con, false);
 goto cleanup;
 }
 
@@ -320,7 +329,7 @@ virConsoleEventOnStdout(int watch ATTRIBUTE_UNUSED,
 if (done < 0) {
 if (errno != EAGAIN) {
 virReportSystemError(errno, "%s", _("cannot write to stdout"));
-virConsoleShutdown(con);
+virConsoleShutdown(con, false);
 }
 goto cleanup;
 }
@@ -342,13 +351,13 @@ virConsoleEventOnStdout(int watch ATTRIBUTE_UNUSED,
 
 if (events & VIR_EVENT_HANDLE_ERROR) {
 

Re: [libvirt] [PATCH] storage_driver: Don't crash in storagePoolCreateXML

[Martin Kletzander]

On Fri, Aug 23, 2019 at 03:24:43PM +0200, Michal Privoznik wrote:

In my recent patches I've introduced
virStoragePoolObjIsStarting() which is then used to protect
storage pool definition when the pool object is locked and
unlocked during long running jobs. Well, my patches did not
anticipate that @obj can be NULL under 'cleanup' label in
storagePoolCreateXML() (for instance when parsing XML fails).
This imperfection is causing libvirtd to crash then.

Fixes: 13284a6b83 storage_driver: Protect pool def during startup and build

Signed-off-by: Michal Privoznik 


Reviewed-by: Martin Kletzander 


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

Re: [libvirt] [PATCH 0/2] rpm: fix usage of 'nc' dep

[Michal Privoznik]

On 8/20/19 11:17 AM, Daniel P. Berrangé wrote:



Daniel P. Berrangé (2):
   rpm: depend on /usr/bin/nc instead of nc
   rpm: move nc dep into the libvirt-daemon sub-RPM

  libvirt.spec.in | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)



Reviewed-by: Michal Privoznik 

Michal

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

[libvirt] [PATCH] storage_driver: Don't crash in storagePoolCreateXML

[Michal Privoznik]

In my recent patches I've introduced
virStoragePoolObjIsStarting() which is then used to protect
storage pool definition when the pool object is locked and
unlocked during long running jobs. Well, my patches did not
anticipate that @obj can be NULL under 'cleanup' label in
storagePoolCreateXML() (for instance when parsing XML fails).
This imperfection is causing libvirtd to crash then.

Fixes: 13284a6b83 storage_driver: Protect pool def during startup and build

Signed-off-by: Michal Privoznik 
---
 src/storage/storage_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index cd9f14a2c0..30940b5dcf 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -808,7 +808,7 @@ storagePoolCreateXML(virConnectPtr conn,
 pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL);
 
  cleanup:
-if (virStoragePoolObjIsStarting(obj)) {
+if (obj && virStoragePoolObjIsStarting(obj)) {
 if (!virStoragePoolObjIsActive(obj))
 virStoragePoolUpdateInactive(obj);
 virStoragePoolObjSetStarting(obj, false);
-- 
2.21.0

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

Re: [libvirt] [PATCH v3] tools: console: Relax stream EOF handling

[Michal Privoznik]

On 8/21/19 3:33 PM, Roman Bolshakov wrote:

Regular VM shutdown triggers the error for existing session of virsh
console and it returns with non-zero exit code:
   error: internal error: console stream EOF

The message and status code are misleading because there's no real
error. virStreamRecv returns 0 correctly when EOF is reached.

Existing implementations of esx, fd, and remote streams behave the same
for virStreamFinish and virStreamAbort: they close the stream. So, we
can continue to use virStreamAbort to handle EOF and errors from
virStreamRecv but additonally we can report error if virStreamAbort
fails.

Fixes: 29f2b5248c6 ("tools: console: pass stream/fd errors to user")
Signed-off-by: Roman Bolshakov 
---
  tools/virsh-console.c | 8 +++-
  1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/tools/virsh-console.c b/tools/virsh-console.c
index e16f841e57..a235a9a283 100644
--- a/tools/virsh-console.c
+++ b/tools/virsh-console.c
@@ -106,7 +106,9 @@ virConsoleShutdown(virConsolePtr con)
  
  if (con->st) {

  virStreamEventRemoveCallback(con->st);
-virStreamAbort(con->st);
+if (virStreamAbort(con->st) < 0)
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("cannot terminate console stream"));
  virStreamFree(con->st);
  con->st = NULL;
  }
@@ -172,10 +174,6 @@ virConsoleEventOnStream(virStreamPtr st,
  if (got == -2)
  goto cleanup; /* blocking */
  if (got <= 0) {
-if (got == 0)
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-   _("console stream EOF"));
-


I still think we need to call virStreamFinish() in this case and not 
virStreamAbort() (hidden in virConsoleShutdown()). The reason is that 
when we read 0 bytes (= indication of EOF) the virStreamFinish() informs 
the sending side that we've read all the data and we've done so 
successfuly. If we abort the stream, it's sending a message that we've 
encountered an error on the very last packet in the stream, which is not 
true. It may not be that huge of the problem in case of ESX where the 
only difference between streamFinish() and streamAbort() is an error 
message that is or is not logged.



However, I will merge your patch because it's removing spurious error 
message and I'll post a patch to allow graceful console shutdown.


Reviewed-by: Michal Privoznik 

Michal

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

Re: [libvirt] [PATCH v1 10/11] storage_driver: Protect pool def during startup and build

[John Ferlan]

On 5/24/19 10:35 AM, Michal Privoznik wrote:
> In near future the storage pool object lock will be released
> during startPool and buildPool callback (in some backends). But
> this means that another thread may acquire the pool object lock
> and change its definition rendering the former thread access not
> only stale definition but also access freed memory
> (virStoragePoolObjAssignDef() will free old def when setting a
> new one).
> 
> One way out of this would be to have the pool appear as active
> because our code deals with obj->def and obj->newdef just fine.
> But we can't declare a pool as active if it's not started or
> still building up. Therefore, have a boolean flag that is very
> similar and forces virStoragePoolObjAssignDef() to store new
> definition in obj->newdef even for an inactive pool. In turn, we
> have to move the definition to correct place when unsetting the
> flag. But that's as easy as calling
> virStoragePoolUpdateInactive().
> 
> Technically speaking, change made to
> storageDriverAutostartCallback() is not needed because until
> storage driver is initialized no storage API can run therefore
> there can't be anyone wanting to change the pool's definition.
> But I'm doing the change there for consistency anyways.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/conf/virstorageobj.c | 26 +-
>  src/conf/virstorageobj.h |  6 ++
>  src/libvirt_private.syms |  2 ++
>  src/storage/storage_driver.c | 31 ++-
>  4 files changed, 63 insertions(+), 2 deletions(-)
> 

[...]

> diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
> index def4123b82..60bfa48e25 100644
> --- a/src/storage/storage_driver.c
> +++ b/src/storage/storage_driver.c
> @@ -201,12 +201,14 @@ storageDriverAutostartCallback(virStoragePoolObjPtr obj,
>  
>  if (virStoragePoolObjIsAutostart(obj) &&
>  !virStoragePoolObjIsActive(obj)) {
> +
> +virStoragePoolObjSetStarting(obj, true);
>  if (backend->startPool &&
>  backend->startPool(obj) < 0) {
>  virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Failed to autostart storage pool '%s': %s"),
> def->name, virGetLastErrorMessage());
> -return;
> +goto cleanup;
>  }
>  started = true;
>  }
> @@ -225,6 +227,13 @@ storageDriverAutostartCallback(virStoragePoolObjPtr obj,
>  virStoragePoolObjSetActive(obj, true);
>  }
>  }
> +
> + cleanup:
> +if (virStoragePoolObjIsStarting(obj)) {
> +if (!virStoragePoolObjIsActive(obj))
> +virStoragePoolUpdateInactive(obj);
> +virStoragePoolObjSetStarting(obj, false);
> +}
>  }
>  
>  
> @@ -750,6 +759,8 @@ storagePoolCreateXML(virConnectPtr conn,
>  newDef = NULL;
>  def = virStoragePoolObjGetDef(obj);
>  

Coverity let me know this morning that there's quite a few lines above
here which goto cleanup; however, cleanup expects @obj != NULL (or at
least virStoragePoolObjIsStarting does). Probably need similar logic
like you added in storagePool{Create|Build}.

John

> +virStoragePoolObjSetStarting(obj, true);
> +
>  if (backend->buildPool) {
>  if (flags & VIR_STORAGE_POOL_CREATE_WITH_BUILD_OVERWRITE)
>  build_flags |= VIR_STORAGE_POOL_BUILD_OVERWRITE;
> @@ -786,6 +797,11 @@ storagePoolCreateXML(virConnectPtr conn,
>  pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL);
>  
>   cleanup:
> +if (virStoragePoolObjIsStarting(obj)) {
> +if (!virStoragePoolObjIsActive(obj))
> +virStoragePoolUpdateInactive(obj);
> +virStoragePoolObjSetStarting(obj, false);
> +}
>  virObjectEventStateQueue(driver->storageEventState, event);
>  virStoragePoolObjEndAPI();
>  return pool;

[...]

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

Re: [libvirt] RFC: revival of hotplug/unplug for PCI Multifunction devices in QEMU guests

[Daniel Henrique Barboza]

On 8/23/19 7:28 AM, Daniel P. Berrangé wrote:

On Thu, Aug 22, 2019 at 05:09:10PM -0300, Daniel Henrique Barboza wrote:

Hi Daniel,

On 6/19/19 4:31 AM, Daniel P. Berrangé wrote:

On Tue, Jun 18, 2019 at 03:04:40PM -0300, Daniel Henrique Barboza wrote:
[...]

Finally had the time to look into it in QEMU. The reason for the difference
is indeed architectural - x86 handles it in slot-granularity level and PPC64
handles it in function level. This is why in x86 unplugging any function
will
remove the whole slot, while in PPC64 each non-zero function needs
to be detached first.

I've sent a patch to QEMU to change the way PPC64 behaves in function
zero unplug [1]. Instead of bail out claiming that there are non-zero
functions left, QEMU will simply remove the remaining functions by itself.
This behavior happens only with function zero unplug (at least in this first
implementation). Not sure if QEMU will accept it, but if it does, it'll
spare us some patches in Libvirt unplug code for PCI Multifunction.
Let's see how it goes.

Looks like the ppc maintainer has queued the patch, which is good.


Yes, that will reduce the amount of patches needed for the hot
unplug code for both archs (since x86 was being impacted by
how ppc64 does the unplug as well). I'll work in resubmit the
patch series considering the upstream PPC64 change.


Thanks,


DHB



Regards,
Daniel


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

Re: [libvirt] RFC: revival of hotplug/unplug for PCI Multifunction devices in QEMU guests

[Daniel P . Berrangé]

On Thu, Aug 22, 2019 at 05:09:10PM -0300, Daniel Henrique Barboza wrote:
> Hi Daniel,
> 
> On 6/19/19 4:31 AM, Daniel P. Berrangé wrote:
> > On Tue, Jun 18, 2019 at 03:04:40PM -0300, Daniel Henrique Barboza wrote:
> > > Hi,
> > > 
> > > This is labeled as RFC but it's more like a FYI to let people know and
> > > comment beforehand. Shiva sent a 28 patch series last year that implements
> > > hotplug/unplug support for PCI multifunction devices [1]. The design
> > > motivation of his work was based in a RFC sent to this mailing list back
> > > in 2016 [2].
> > > 
> > > I'll briefly summarize the goals and motivations here. What we have today
> > > in Libvirt:
> > > 
> > > - no hotplug/unplug support for multifunction PCI devices
> > > 
> > > This is explained in details in [2]. When hotplugging a multifunction
> > > device, QEMU will queue the hotplug operation of all non-zero functions 
> > > and,
> > > when function 0 is hotplugged, all functions are hotplugged together. This
> > > is true for all archs that supports PCI multifunction devices in QEMU. For
> > > unplug it varies: x86 will unplug all functions if any function is
> > > unplugged, ppc64 needs to unplug each one.
> > Do you know anything about why ppc64 & x86 are different in this respect
> > in QEMU.  I think it would be desirable to fix QEMU so that unplug works
> > consistently across architectures. These kind of behavioural differences
> > are a cause of pain as x86 gets all the day to day testing & leaving
> > ppc64 to bitrot if it behaves differently.
> 
> Finally had the time to look into it in QEMU. The reason for the difference
> is indeed architectural - x86 handles it in slot-granularity level and PPC64
> handles it in function level. This is why in x86 unplugging any function
> will
> remove the whole slot, while in PPC64 each non-zero function needs
> to be detached first.
> 
> I've sent a patch to QEMU to change the way PPC64 behaves in function
> zero unplug [1]. Instead of bail out claiming that there are non-zero
> functions left, QEMU will simply remove the remaining functions by itself.
> This behavior happens only with function zero unplug (at least in this first
> implementation). Not sure if QEMU will accept it, but if it does, it'll
> spare us some patches in Libvirt unplug code for PCI Multifunction.
> Let's see how it goes.

Looks like the ppc maintainer has queued the patch, which is good.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

[libvirt] [PATCH] virpci: Rename virPCIDevice{Bind, Unbind}FromStubWithOverride

[Michal Privoznik]

After my previous patches we have virPCIDeviceBindToStub() and
virPCIDeviceUnbindFromStub() which really do nothing but call
virPCIDeviceBindToStubWithOverride() and
virPCIDeviceUnbindFromStubWithOverride() respectively.
Drop "WithOverride" from the names and drop the thin wrappers.

Signed-off-by: Michal Privoznik 
---

Pushed under trivial rule.

 src/util/virpci.c | 16 ++--
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/src/util/virpci.c b/src/util/virpci.c
index 6724a8ad9e..ee78151e74 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1136,7 +1136,7 @@ virPCIDeviceBindWithDriverOverride(virPCIDevicePtr dev,
 }
 
 static int
-virPCIDeviceUnbindFromStubWithOverride(virPCIDevicePtr dev)
+virPCIDeviceUnbindFromStub(virPCIDevicePtr dev)
 {
 if (!dev->unbind_from_stub) {
 VIR_DEBUG("Unbind from stub skipped for PCI device %s", dev->name);
@@ -1147,13 +1147,7 @@ virPCIDeviceUnbindFromStubWithOverride(virPCIDevicePtr 
dev)
 }
 
 static int
-virPCIDeviceUnbindFromStub(virPCIDevicePtr dev)
-{
-return virPCIDeviceUnbindFromStubWithOverride(dev);
-}
-
-static int
-virPCIDeviceBindToStubWithOverride(virPCIDevicePtr dev)
+virPCIDeviceBindToStub(virPCIDevicePtr dev)
 {
 const char *stubDriverName;
 VIR_AUTOFREE(char *) stubDriverPath = NULL;
@@ -1192,12 +1186,6 @@ virPCIDeviceBindToStubWithOverride(virPCIDevicePtr dev)
 return 0;
 }
 
-static int
-virPCIDeviceBindToStub(virPCIDevicePtr dev)
-{
-return virPCIDeviceBindToStubWithOverride(dev);
-}
-
 /* virPCIDeviceDetach:
  *
  * Detach this device from the host driver, attach it to the stub
-- 
2.21.0

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

Re: [libvirt] [PATCH 0/2] rpm: fix usage of 'nc' dep

[Daniel P . Berrangé]

ping

On Tue, Aug 20, 2019 at 10:17:15AM +0100, Daniel P. Berrangé wrote:
> 
> 
> Daniel P. Berrangé (2):
>   rpm: depend on /usr/bin/nc instead of nc
>   rpm: move nc dep into the libvirt-daemon sub-RPM
> 
>  libvirt.spec.in | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> -- 
> 2.21.0
> 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

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

Re: [libvirt] [PATCH 2/2] qapi: deprecate implicit filters

[Vladimir Sementsov-Ogievskiy]

14.08.2019 13:07, Vladimir Sementsov-Ogievskiy wrote:
> To get rid of implicit filters related workarounds in future let's
> deprecate them now.

Interesting, could we deprecate implicit filter without deprecation of 
unnecessity of
parameter? As actually, it's good when this parameter is not necessary, in most 
cases
user is not interested in node-name.

Obviously we can do the following:

1. In 4.2 we deprecate unnecessity, which implies deprecation of implicit 
filters
2. After some releases in 4.x we can drop deprecated functionality, so we drop 
it together with
implicit filters. And, in same release 4.x we return it back (as it's 
compatible change :)
but without implicit filters (so, if filter-node-name not specified, we just 
create
explicit filter with autogenerated node-name)

So, effectively we just drop "deprecation mark" together with implicit filters, 
which is nice
but actually confusing.

Instead, we may do
1. In 4.2 deprecate
2. In 4.x drop optionality together with implicit filters
3. In 4.y (y > x of course) return optionality back

It's a bit safer, but for users who miss releases [4.x, 4.y) it's no 
difference..

Or we just write in spec, that implicit filters are deprecated? But we have 
nothing about implicit
filters in spec. More over, we directly write that we have filter, and if 
parameter is omitted
it's node-name is autogenerated. So actually, the fact the filter is hidden 
when filter-node-name is
unspecified is _undocumented_.

So, finally, it looks like nothing to deprecated in specification, we can just 
drop implicit filters :)

What do you think?

> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>   qemu-deprecated.texi  |  7 +++
>   qapi/block-core.json  |  6 --
>   include/block/block_int.h | 10 +-
>   blockdev.c| 10 ++
>   4 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> index 2753fafd0b..8222440148 100644
> --- a/qemu-deprecated.texi
> +++ b/qemu-deprecated.texi
> @@ -183,6 +183,13 @@ the 'wait' field, which is only applicable to sockets in 
> server mode
>   
>   Use blockdev-mirror and blockdev-backup instead.
>   
> +@subsection implicit filters (since 4.2)
> +
> +Mirror and commit jobs inserts filters, which becomes implicit if user
> +omitted filter-node-name parameter. So omitting it is deprecated, set it
> +always. Note, that drive-mirror don't have this parameter, so it will
> +create implicit filter anyway, but drive-mirror is deprecated itself too.
> +
>   @section Human Monitor Protocol (HMP) commands
>   
>   @subsection The hub_id parameter of 'hostfwd_add' / 'hostfwd_remove' (since 
> 3.1)
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 4e35526634..0505ac9d8b 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1596,7 +1596,8 @@
>   # @filter-node-name: the node name that should be assigned to the
>   #filter driver that the commit job inserts into the 
> graph
>   #above @top. If this option is not given, a node name is
> -#autogenerated. (Since: 2.9)
> +#autogenerated. Omitting this option is deprecated, it 
> will
> +#be required in future. (Since: 2.9)
>   #
>   # @auto-finalize: When false, this job will wait in a PENDING state after 
> it has
>   # finished its work, waiting for @block-job-finalize before
> @@ -2249,7 +2250,8 @@
>   # @filter-node-name: the node name that should be assigned to the
>   #filter driver that the mirror job inserts into the 
> graph
>   #above @device. If this option is not given, a node 
> name is
> -#autogenerated. (Since: 2.9)
> +#autogenerated. Omitting this option is deprecated, it 
> will
> +#be required in future. (Since: 2.9)
>   #
>   # @copy-mode: when to copy data to the destination; defaults to 'background'
>   # (Since: 3.0)
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 3aa1e832a8..624da0b4a2 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -762,7 +762,15 @@ struct BlockDriverState {
>   bool sg;/* if true, the device is a /dev/sg* */
>   bool probed;/* if true, format was probed rather than specified */
>   bool force_share; /* if true, always allow all shared permissions */
> -bool implicit;  /* if true, this filter node was automatically inserted 
> */
> +
> +/*
> + * @implicit field is deprecated, don't set it to true for new filters.
> + * If true, this filter node was automatically inserted and user don't
> + * know about it and unprepared for any effects of it. So, implicit
> + * filters are workarounded and skipped in many places of the block
> + * layer code.
> + */
> +bool implicit;
>   
>   

Re: [libvirt] [PATCH 00/12] Drop KVM assignment

[Michal Privoznik]

On 8/20/19 4:30 PM, Michal Privoznik wrote:
>

Thank you guys for review. I've pushed these.

Michal

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