[libvirt] [PATCH 1/2] util: rename qemuGetProcessInfo to virProcessGetStat

[Wang King]

qemuGetProcessInfo is more likely a process utility function, just rename it
to virProcessGetStat and move it to virprocess.c source file.
---
 src/libvirt_private.syms |  1 +
 src/qemu/qemu_driver.c   | 83 ++--
 src/util/virprocess.c| 62 
 src/util/virprocess.h|  4 +++
 4 files changed, 83 insertions(+), 67 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d361454..3681869 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2382,6 +2382,7 @@ virProcessGetMaxMemLock;
 virProcessGetNamespaces;
 virProcessGetPids;
 virProcessGetStartTime;
+virProcessGetStat;
 virProcessKill;
 virProcessKillPainfully;
 virProcessNamespaceAvailable;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6c79d4f..a4aa5da 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1378,68 +1378,6 @@ qemuGetSchedInfo(unsigned long long *cpuWait,
 return ret;
 }
 
-
-static int
-qemuGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss,
-   pid_t pid, int tid)
-{
-char *proc;
-FILE *pidinfo;
-unsigned long long usertime = 0, systime = 0;
-long rss = 0;
-int cpu = 0;
-int ret;
-
-/* In general, we cannot assume pid_t fits in int; but /proc parsing
- * is specific to Linux where int works fine.  */
-if (tid)
-ret = virAsprintf(, "/proc/%d/task/%d/stat", (int) pid, tid);
-else
-ret = virAsprintf(, "/proc/%d/stat", (int) pid);
-if (ret < 0)
-return -1;
-
-pidinfo = fopen(proc, "r");
-VIR_FREE(proc);
-
-/* See 'man proc' for information about what all these fields are. We're
- * only interested in a very few of them */
-if (!pidinfo ||
-fscanf(pidinfo,
-   /* pid -> stime */
-   "%*d (%*[^)]) %*c %*d %*d %*d %*d %*d %*u %*u %*u %*u %*u %llu 
%llu"
-   /* cutime -> endcode */
-   "%*d %*d %*d %*d %*d %*d %*u %*u %ld %*u %*u %*u"
-   /* startstack -> processor */
-   "%*u %*u %*u %*u %*u %*u %*u %*u %*u %*u %*d %d",
-   , , , ) != 4) {
-VIR_WARN("cannot parse process status data");
-}
-
-/* We got jiffies
- * We want nanoseconds
- * _SC_CLK_TCK is jiffies per second
- * So calculate thus
- */
-if (cpuTime)
-*cpuTime = 1000ull * 1000ull * 1000ull * (usertime + systime)
-/ (unsigned long long)sysconf(_SC_CLK_TCK);
-if (lastCpu)
-*lastCpu = cpu;
-
-if (vm_rss)
-*vm_rss = rss * virGetSystemPageSizeKB();
-
-
-VIR_DEBUG("Got status for %d/%d user=%llu sys=%llu cpu=%d rss=%ld",
-  (int) pid, tid, usertime, systime, cpu, rss);
-
-VIR_FORCE_FCLOSE(pidinfo);
-
-return 0;
-}
-
-
 static int
 qemuDomainHelperGetVcpus(virDomainObjPtr vm,
  virVcpuInfoPtr info,
@@ -1482,9 +1420,9 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm,
 vcpuinfo->number = i;
 vcpuinfo->state = VIR_VCPU_RUNNING;
 
-if (qemuGetProcessInfo(>cpuTime,
-   >cpu, NULL,
-   vm->pid, vcpupid) < 0) {
+if (virProcessGetStat(vm->pid, vcpupid,
+  >cpuTime,
+  >cpu, NULL) < 0) {
 virReportSystemError(errno, "%s",
  _("cannot get vCPU placement & pCPU 
time"));
 return -1;
@@ -2641,7 +2579,7 @@ qemuDomainGetInfo(virDomainPtr dom,
 }
 
 if (virDomainObjIsActive(vm)) {
-if (qemuGetProcessInfo(&(info->cpuTime), NULL, NULL, vm->pid, 0) < 0) {
+if (virProcessGetStat(vm->pid, 0, &(info->cpuTime), NULL, NULL) < 0) {
 virReportError(VIR_ERR_OPERATION_FAILED, "%s",
_("cannot read cputime for domain"));
 goto cleanup;
@@ -8172,6 +8110,8 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
 virDomainDeviceDefPtr dev = NULL, dev_copy = NULL;
 bool force = (flags & VIR_DOMAIN_DEVICE_MODIFY_FORCE) != 0;
 int ret = -1;
+virQEMUCapsPtr qemuCaps = NULL;
+qemuDomainObjPrivatePtr priv;
 virQEMUDriverConfigPtr cfg = NULL;
 virCapsPtr caps = NULL;
 unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE;
@@ -8190,6 +8130,8 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
 if (!(vm = qemuDomObjFromDomain(dom)))
 goto cleanup;
 
+priv = vm->privateData;
+
 if (virDomainUpdateDeviceFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
 goto cleanup;
 
@@ -8216,6 +8158,12 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
 goto endjob;
 }
 
+if (priv->qemuCaps)
+qemuCaps = virObjectRef(priv->qemuCaps);
+else if (!(qemuCaps = virQEMUCapsCacheLookup(caps, driver->qemuCapsCache,
+  

[libvirt] [PATCH 2/2] util: rename qemuGetSchedInfo to virProcessGetSchedInfo

[Wang King]

qemuGetSchedInfo is more likely a process utility function, just rename it
to virProcessGetSchedInfo and move it to virprocess.c source file.
---
 src/libvirt_private.syms |  1 +
 src/qemu/qemu_driver.c   | 78 +---
 src/util/virprocess.c| 75 ++
 src/util/virprocess.h|  2 ++
 4 files changed, 79 insertions(+), 77 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3681869..7f468dc 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2381,6 +2381,7 @@ virProcessGetAffinity;
 virProcessGetMaxMemLock;
 virProcessGetNamespaces;
 virProcessGetPids;
+virProcessGetSchedInfo;
 virProcessGetStartTime;
 virProcessGetStat;
 virProcessKill;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a4aa5da..9e2fb7e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1302,82 +1302,6 @@ static char *qemuConnectGetCapabilities(virConnectPtr 
conn) {
 return xml;
 }
 
-
-static int
-qemuGetSchedInfo(unsigned long long *cpuWait,
- pid_t pid, pid_t tid)
-{
-char *proc = NULL;
-char *data = NULL;
-char **lines = NULL;
-size_t i;
-int ret = -1;
-double val;
-
-*cpuWait = 0;
-
-/* In general, we cannot assume pid_t fits in int; but /proc parsing
- * is specific to Linux where int works fine.  */
-if (tid)
-ret = virAsprintf(, "/proc/%d/task/%d/sched", (int)pid, (int)tid);
-else
-ret = virAsprintf(, "/proc/%d/sched", (int)pid);
-if (ret < 0)
-goto cleanup;
-ret = -1;
-
-/* The file is not guaranteed to exist (needs CONFIG_SCHED_DEBUG) */
-if (access(proc, R_OK) < 0) {
-ret = 0;
-goto cleanup;
-}
-
-if (virFileReadAll(proc, (1<<16), ) < 0)
-goto cleanup;
-
-lines = virStringSplit(data, "\n", 0);
-if (!lines)
-goto cleanup;
-
-for (i = 0; lines[i] != NULL; i++) {
-const char *line = lines[i];
-
-/* Needs CONFIG_SCHEDSTATS. The second check
- * is the old name the kernel used in past */
-if (STRPREFIX(line, "se.statistics.wait_sum") ||
-STRPREFIX(line, "se.wait_sum")) {
-line = strchr(line, ':');
-if (!line) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("Missing separator in sched info '%s'"),
-   lines[i]);
-goto cleanup;
-}
-line++;
-while (*line == ' ')
-line++;
-
-if (virStrToDouble(line, NULL, ) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   _("Unable to parse sched info value '%s'"),
-   line);
-goto cleanup;
-}
-
-*cpuWait = (unsigned long long)(val * 100);
-break;
-}
-}
-
-ret = 0;
-
- cleanup:
-VIR_FREE(data);
-VIR_FREE(proc);
-virStringListFree(lines);
-return ret;
-}
-
 static int
 qemuDomainHelperGetVcpus(virDomainObjPtr vm,
  virVcpuInfoPtr info,
@@ -1441,7 +1365,7 @@ qemuDomainHelperGetVcpus(virDomainObjPtr vm,
 }
 
 if (cpuwait) {
-if (qemuGetSchedInfo(&(cpuwait[ncpuinfo]), vm->pid, vcpupid) < 0)
+if (virProcessGetSchedInfo(vm->pid, vcpupid, &(cpuwait[ncpuinfo])) 
< 0)
 return -1;
 }
 
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 98f4b25..67e8cef 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -1488,3 +1488,78 @@ virProcessGetStat(pid_t pid, int tid,
 return 0;
 
 }
+
+int
+virProcessGetSchedInfo(pid_t pid, int tid,
+  unsigned long long *cpuWait)
+{
+char *proc = NULL;
+char *data = NULL;
+char **lines = NULL;
+size_t i;
+int ret = -1;
+double val;
+
+*cpuWait = 0;
+
+/* In general, we cannot assume pid_t fits in int; but /proc parsing
+ * is specific to Linux where int works fine.  */
+if (tid)
+ret = virAsprintf(, "/proc/%d/task/%d/sched", (int)pid, (int)tid);
+else
+ret = virAsprintf(, "/proc/%d/sched", (int)pid);
+if (ret < 0)
+goto cleanup;
+ret = -1;
+
+/* The file is not guaranteed to exist (needs CONFIG_SCHED_DEBUG) */
+if (access(proc, R_OK) < 0) {
+ret = 0;
+goto cleanup;
+}
+
+if (virFileReadAll(proc, (1<<16), ) < 0)
+goto cleanup;
+
+lines = virStringSplit(data, "\n", 0);
+if (!lines)
+goto cleanup;
+
+for (i = 0; lines[i] != NULL; i++) {
+const char *line = lines[i];
+
+/* Needs CONFIG_SCHEDSTATS. The second check
+ * is the old name the kernel used in past */
+if (STRPREFIX(line, "se.statistics.wait_sum") ||
+STRPREFIX(line, "se.wait_sum")) {
+line = strchr(line, 

Re: [libvirt] [PATCH] usb: keep leading zeros of vendor/product id in USB device

[Chen Hanxiao]

At 2017-05-19 15:17:32, "Chen Hanxiao"  wrote:
>From: Chen Hanxiao 
>
>Some vendor id or product id may have leading zeros.
>We should show them.
>
>Signed-off-by: Chen Hanxiao 

ping

Regards,
- Chen

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

Re: [libvirt] [PATCH V2] libxl: add default controllers for USB devices

[Jim Fehlig]

On 05/23/2017 01:11 AM, Cedric Bosdonnat wrote:

On Fri, 2017-05-05 at 15:48 -0600, Jim Fehlig wrote:

Attempting to start a domain with USB hostdevs but no USB controllers
fails with the rather cryptic error

libxl: error: libxl_qmp.c:287:qmp_handle_error_response: received an
error message from QMP server: Bus 'xenusb-0.0' not found

This can be fixed by creating default USB controllers. When no USB
controllers are defined, create the number of 8 port controllers
necessary to accommodate the number of defined USB devices.

Note that USB controllers are already created as needed in the
domainAttachDevice code path. E.g. a USB controller will be created,
if necessary, when attaching a USB device with
'virsh attach-device dom usbdev.xml'.

Signed-off-by: Jim Fehlig 
---

V1 here

https://www.redhat.com/archives/libvir-list/2017-April/msg00965.html

While further testing of V1 found a libvirtd segfault due to
incorrectly using virDomainControllerInsertPreAlloced instead of
virDomainControllerInsert.

  src/libxl/libxl_conf.c | 82 +++---
  1 file changed, 71 insertions(+), 11 deletions(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 56bc09719..cdf6ec9f3 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1822,34 +1822,94 @@ libxlMakeUSBController(virDomainControllerDefPtr 
controller,
  }
  
  static int

+libxlMakeDefaultUSBControllers(virDomainDefPtr def,
+   libxl_domain_config *d_config)
+{
+virDomainControllerDefPtr l_controller = NULL;
+libxl_device_usbctrl *x_controllers = NULL;
+size_t nusbdevs = 0;
+size_t ncontrollers;
+size_t i;
+
+for (i = 0; i < def->nhostdevs; i++) {
+if (def->hostdevs[i]->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
+def->hostdevs[i]->source.subsys.type == 
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
+nusbdevs++;
+}
+
+/* No controllers needed if there are no USB devs */
+if (nusbdevs == 0)
+return 0;
+
+/* Create USB controllers with 8 ports */
+ncontrollers = VIR_DIV_UP(nusbdevs, 8);
+if (VIR_ALLOC_N(x_controllers, ncontrollers) < 0)
+return -1;
+
+for (i = 0; i < ncontrollers; i++) {
+if (!(l_controller = 
virDomainControllerDefNew(VIR_DOMAIN_CONTROLLER_TYPE_USB)))
+goto error;
+
+l_controller->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB2;
+l_controller->idx = i;
+l_controller->opts.usbopts.ports = 8;
+
+libxl_device_usbctrl_init(_controllers[i]);
+
+if (libxlMakeUSBController(l_controller, _controllers[i]) < 0)
+goto error;
+
+if (virDomainControllerInsert(def, l_controller) < 0)
+goto error;
+
+l_controller = NULL;
+}
+
+d_config->usbctrls = x_controllers;
+d_config->num_usbctrls = ncontrollers;
+return 0;
+
+ error:
+ virDomainControllerDefFree(l_controller);
+ for (i = 0; i < ncontrollers; i++)
+ libxl_device_usbctrl_dispose(_controllers[i]);
+ VIR_FREE(x_controllers);
+ return -1;
+}
+
+static int
  libxlMakeUSBControllerList(virDomainDefPtr def, libxl_domain_config *d_config)
  {
  virDomainControllerDefPtr *l_controllers = def->controllers;
  size_t ncontrollers = def->ncontrollers;
  size_t nusbctrls = 0;
  libxl_device_usbctrl *x_usbctrls;
-size_t i;
-
-if (ncontrollers == 0)
-return 0;
-
-if (VIR_ALLOC_N(x_usbctrls, ncontrollers) < 0)
-return -1;
+size_t i, j;
  
  for (i = 0; i < ncontrollers; i++) {

+if (l_controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_USB)
+nusbctrls++;
+}
+
+if (nusbctrls == 0)
+return libxlMakeDefaultUSBControllers(def, d_config);
+
+if (VIR_ALLOC_N(x_usbctrls, nusbctrls) < 0)
+return -1;
+
+for (i = 0, j = 0; i < ncontrollers && j < nusbctrls; i++) {
  if (l_controllers[i]->type != VIR_DOMAIN_CONTROLLER_TYPE_USB)
  continue;
  
-libxl_device_usbctrl_init(_usbctrls[nusbctrls]);

+libxl_device_usbctrl_init(_usbctrls[j]);
  
  if (libxlMakeUSBController(l_controllers[i],

-   _usbctrls[nusbctrls]) < 0)
+   _usbctrls[j]) < 0)
  goto error;
  
-nusbctrls++;

+j++;
  }
  
-VIR_SHRINK_N(x_usbctrls, ncontrollers, ncontrollers - nusbctrls);

  d_config->usbctrls = x_usbctrls;
  d_config->num_usbctrls = nusbctrls;
  


ACK


Cool, thanks for taking a look! Pushed now.

Regards,
Jim

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

Re: [libvirt] [PATCH] qemu: Properly check return value of VIR_STRDUP in qemuDomainGetBlockIoTune

[Jiri Denemark]

On Tue, May 23, 2017 at 17:40:13 +0200, Peter Krempa wrote:
> Setting the 'group_name' for a disk would falsely trigger a error path

s/a error/an error/

> as in commit 4b57f76502 we did not properly check the return value of
> VIR_STRDUP.
> ---
> I must have forgotten to commit this change, as I'm sure I've tested it prior
> to sending the patch mentioned above.
> 
>  src/qemu/qemu_driver.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 6c79d4fe3..cd513ff9f 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -17748,7 +17748,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
> 
>  /* Group name needs to be copied since qemuMonitorGetBlockIoThrottle
>   * allocates it as well */
> -if (VIR_STRDUP(reply.group_name, disk->blkdeviotune.group_name))
> +if (VIR_STRDUP(reply.group_name, disk->blkdeviotune.group_name) < 0)
>  goto endjob;
>  }

ACK

Jirka

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

[libvirt] Various apparmor related changes (part 2)

[Stefan Bader]

> Over the years there have been a bunch of changes to the
> apparmor profiles and/or virt-aa-helper which have been
> carried in Debian/Ubuntu but never made it upstream.
> 
> In an attempt to clean this up and generally improve the
> apparmor based environments, we (Christian and I) went
> over the changes, cleaned out cruft as much as possible 
> and would be sending out hunks of changes to this list
> for upstream inclusion.
> 

This second batch consists partially of some reworked
patches from the previous round and some more things
which hopefully are simple enough and improve the upstream
profiles.

5+6: Although these are Debian/Ubuntu specific, there are
 other paths which are specific for SuSE. So I wondered
 why not have both upstream.
9:   Jamie, I know it has been a long time but do you remember
 what this resolved?

Thanks,
Stefan

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

[libvirt] [PATCH 01/10] virt-aa-helper, apparmor: allow /usr/share/OVMF/ too

[Stefan Bader]

From: Simon McVittie 

The split firmware and variables files introduced by
https://bugs.debian.org/764918 are in a different directory for
some reason. Let the virtual machine read both.

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
---
 examples/apparmor/libvirt-qemu | 1 +
 src/security/virt-aa-helper.c  | 1 +
 tests/virt-aa-helper-test  | 7 ++-
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
index a9020aa..e0988bb 100644
--- a/examples/apparmor/libvirt-qemu
+++ b/examples/apparmor/libvirt-qemu
@@ -70,6 +70,7 @@
   /usr/share/vgabios/** r,
   /usr/share/seabios/** r,
   /usr/share/ovmf/** r,
+  /usr/share/OVMF/** r,
 
   # access PKI infrastructure
   /etc/pki/libvirt-vnc/** r,
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 5f5d1cd..6c5fc28 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -512,6 +512,7 @@ valid_path(const char *path, const bool readonly)
 "/vmlinuz",
 "/initrd",
 "/initrd.img",
+"/usr/share/OVMF/",  /* for OVMF images */
 "/usr/share/ovmf/"   /* for OVMF images */
 };
 /* override the above with these */
diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test
index 68e9399..c05afc1 100755
--- a/tests/virt-aa-helper-test
+++ b/tests/virt-aa-helper-test
@@ -296,8 +296,13 @@ if [ -f /usr/share/ovmf/OVMF.fd ]; then
 -e "s,###DISK###,$disk1,g" \
 -e "s,,/usr/share/ovmf/OVMF.fd,g" "$template_xml" > 
"$test_xml"
 testme "0" "ovmf" "-r -u $valid_uuid" "$test_xml"
+elif [ -f /usr/share/OVMF/OVMF.fd ]; then
+sed -e "s,###UUID###,$uuid,g"  \
+-e "s,###DISK###,$disk1,g" \
+-e "s,,/usr/share/OVMF/OVMF.fd,g" "$template_xml" > 
"$test_xml"
+testme "0" "ovmf" "-r -u $valid_uuid" "$test_xml"
 else
-echo "Skipping OVMF test. Could not find /usr/share/ovmf/OVMF.fd"
+echo "Skipping OVMF test. Could not find /usr/share/ovmf/OVMF.fd or 
/usr/share/OVMF/OVMF.fd"
 fi
 
 sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e 
"s,,$tmpdir/initrd,g" "$template_xml" > "$test_xml"
-- 
2.7.4

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

[libvirt] [PATCH 02/10] virt-aa-helper: Generalize test for firmware paths

[Stefan Bader]

From: Christian Ehrhardt 

This replaces individual tests for firmware locations by
a generic function which will simplify having additional
locations in the future.

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
---
 tests/virt-aa-helper-test | 29 -
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test
index c05afc1..73f3080 100755
--- a/tests/virt-aa-helper-test
+++ b/tests/virt-aa-helper-test
@@ -145,6 +145,20 @@ testme() {
 fi
 }
 
+testfw() {
+title="$1"
+fwpath="$2"
+
+if [ -f "$fwpath" ]; then
+sed -e "s,###UUID###,$uuid,g"  \
+-e "s,###DISK###,$disk1,g" \
+-e "s,,$fwpath,g" "$template_xml" > "$test_xml"
+testme "0" "$title" "-r -u $valid_uuid" "$test_xml"
+else
+echo "Skipping FW $title test. Could not find $fwpath"
+fi
+}
+
 # Expected failures
 echo "Expected failures:" >$output
 testme "1" "invalid arg" "-z"
@@ -291,19 +305,8 @@ sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" 
-e "s,,$tm
 touch "$tmpdir/kernel"
 testme "0" "kernel" "-r -u $valid_uuid" "$test_xml"
 
-if [ -f /usr/share/ovmf/OVMF.fd ]; then
-sed -e "s,###UUID###,$uuid,g"  \
--e "s,###DISK###,$disk1,g" \
--e "s,,/usr/share/ovmf/OVMF.fd,g" "$template_xml" > 
"$test_xml"
-testme "0" "ovmf" "-r -u $valid_uuid" "$test_xml"
-elif [ -f /usr/share/OVMF/OVMF.fd ]; then
-sed -e "s,###UUID###,$uuid,g"  \
--e "s,###DISK###,$disk1,g" \
--e "s,,/usr/share/OVMF/OVMF.fd,g" "$template_xml" > 
"$test_xml"
-testme "0" "ovmf" "-r -u $valid_uuid" "$test_xml"
-else
-echo "Skipping OVMF test. Could not find /usr/share/ovmf/OVMF.fd or 
/usr/share/OVMF/OVMF.fd"
-fi
+testfw "ovmf (old path)" "/usr/share/ovmf/OVMF.fd"
+testfw "OVMF (new path)" "/usr/share/OVMF/OVMF_CODE.fd"
 
 sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e 
"s,,$tmpdir/initrd,g" "$template_xml" > "$test_xml"
 touch "$tmpdir/initrd"
-- 
2.7.4

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

[libvirt] [PATCH 09/10] apparmor, libvirt-qemu: Allow read access to overcommit_memory

[Stefan Bader]

From: Jamie Strandboge 

Allow qemu to read @{PROC}/sys/vm/overcommit_memory.

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
---
 examples/apparmor/libvirt-qemu | 1 +
 1 file changed, 1 insertion(+)

diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
index e2b0dfd..89525b3 100644
--- a/examples/apparmor/libvirt-qemu
+++ b/examples/apparmor/libvirt-qemu
@@ -26,6 +26,7 @@
   # only modify its comm value or those in its thread group.
   owner @{PROC}/@{pid}/task/@{tid}/comm rw,
   @{PROC}/sys/kernel/cap_last_cap r,
+  @{PROC}/sys/vm/overcommit_memory r,
 
   # For hostdev access. The actual devices will be added dynamically
   /sys/bus/usb/devices/ r,
-- 
2.7.4

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

[libvirt] [PATCH 10/10] apparmor, libvirt-qemu: Allow access to certificates used by libvirt-vnc

[Stefan Bader]

From: Serge Hallyn 

When setting up VncTLS according to the official Libvirt documentation,
only one certificate for libvirt/libvirt-vnc is used. The document
indicates to use the following directories :

 /etc/pki/CA
 /etc/pki/libvirt
 /etc/pki/libvirt/private

in order to manage the certificates used by libvirt-vnc.

Bug-Ubuntu: https://bugs.launchpad.net/bugs/901272

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
---
 examples/apparmor/libvirt-qemu | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
index 89525b3..e990ab4 100644
--- a/examples/apparmor/libvirt-qemu
+++ b/examples/apparmor/libvirt-qemu
@@ -144,6 +144,12 @@
   /usr/{lib,lib64}/qemu/block-curl.so mr,
   /usr/{lib,lib64}/qemu/block-rbd.so mr,
 
+  # for use by libvirt-vnc (LP: #901272)
+  /etc/pki/CA/ r,
+  /etc/pki/CA/* r,
+  /etc/pki/libvirt/ r,
+  /etc/pki/libvirt/** r,
+
   # for save and resume
   /{usr/,}bin/dash rmix,
   /{usr/,}bin/dd rmix,
-- 
2.7.4

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

[libvirt] [PATCH 03/10] apparmor, virt-aa-helper: Allow aarch64 UEFI.

[Stefan Bader]

From: William Grant 

Allow access to aarch64 UEFI images.

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
Acked-by: Guido Günther 
---
 examples/apparmor/libvirt-qemu | 2 ++
 src/security/virt-aa-helper.c  | 4 +++-
 tests/virt-aa-helper-test  | 2 ++
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
index e0988bb..89466c9 100644
--- a/examples/apparmor/libvirt-qemu
+++ b/examples/apparmor/libvirt-qemu
@@ -71,6 +71,8 @@
   /usr/share/seabios/** r,
   /usr/share/ovmf/** r,
   /usr/share/OVMF/** r,
+  /usr/share/AAVMF/** r,
+  /usr/share/qemu-efi/** r,
 
   # access PKI infrastructure
   /etc/pki/libvirt-vnc/** r,
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 6c5fc28..69e797c 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -513,7 +513,9 @@ valid_path(const char *path, const bool readonly)
 "/initrd",
 "/initrd.img",
 "/usr/share/OVMF/",  /* for OVMF images */
-"/usr/share/ovmf/"   /* for OVMF images */
+"/usr/share/ovmf/",  /* for OVMF images */
+"/usr/share/AAVMF/", /* for AAVMF images */
+"/usr/share/qemu-efi/"   /* for AAVMF images */
 };
 /* override the above with these */
 const char * const override[] = {
diff --git a/tests/virt-aa-helper-test b/tests/virt-aa-helper-test
index 73f3080..51072f6 100755
--- a/tests/virt-aa-helper-test
+++ b/tests/virt-aa-helper-test
@@ -307,6 +307,8 @@ testme "0" "kernel" "-r -u $valid_uuid" "$test_xml"
 
 testfw "ovmf (old path)" "/usr/share/ovmf/OVMF.fd"
 testfw "OVMF (new path)" "/usr/share/OVMF/OVMF_CODE.fd"
+testfw "AAVMF" "/usr/share/AAVMF/AAVMF_CODE.fd"
+testfw "qemu-efi" "/usr/share/qemu-efi/QEMU_EFI.fd"
 
 sed -e "s,###UUID###,$uuid,g" -e "s,###DISK###,$disk1,g" -e 
"s,,$tmpdir/initrd,g" "$template_xml" > "$test_xml"
 touch "$tmpdir/initrd"
-- 
2.7.4

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

[libvirt] [PATCH 04/10] apparmor, libvirt-qemu: Add ppc64el related changes

[Stefan Bader]

From: Serge Hallyn 

Updates profile to allow running on ppc64el.

Bug-Ubuntu: https://bugs.launchpad.net/bugs/1374554

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
---
 examples/apparmor/libvirt-qemu | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
index 89466c9..7fa512f 100644
--- a/examples/apparmor/libvirt-qemu
+++ b/examples/apparmor/libvirt-qemu
@@ -73,6 +73,7 @@
   /usr/share/OVMF/** r,
   /usr/share/AAVMF/** r,
   /usr/share/qemu-efi/** r,
+  /usr/share/slof/** r,
 
   # access PKI infrastructure
   /etc/pki/libvirt-vnc/** r,
@@ -154,3 +155,8 @@
   /etc/udev/udev.conf r,
   /sys/bus/ r,
   /sys/class/ r,
+
+  # for ppc device-tree access
+  @{PROC}/device-tree/ r,
+  @{PROC}/device-tree/** r,
+  /sys/firmware/devicetree/** r,
-- 
2.7.4

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

[libvirt] [PATCH 06/10] apparmor, libvirtd: Allow libxl-save-helper to run on Debian/Ubuntu

[Stefan Bader]

On Debian/Ubuntu the libxl-save-helper (used when saving/restoring
a domain through libxl) is located under /usr/lib/xen-/bin.

Bug-Ubuntu: https://bugs.launchpad.net/bugs/1334195

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
---
 examples/apparmor/usr.sbin.libvirtd | 1 +
 1 file changed, 1 insertion(+)

diff --git a/examples/apparmor/usr.sbin.libvirtd 
b/examples/apparmor/usr.sbin.libvirtd
index f43bfd5..64f6d2c 100644
--- a/examples/apparmor/usr.sbin.libvirtd
+++ b/examples/apparmor/usr.sbin.libvirtd
@@ -51,6 +51,7 @@
   /usr/{lib,lib64}/xen-common/bin/xen-toolstack PUx,
   /usr/{lib,lib64}/xen/bin/* Ux,
   /usr/lib/xen-*/bin/pygrub PUx,
+  /usr/lib/xen-*/bin/libxl-save-helper PUx,
 
   # force the use of virt-aa-helper
   audit deny /{usr/,}sbin/apparmor_parser rwxl,
-- 
2.7.4

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

[libvirt] [PATCH 07/10] apparmor, libvirt-qemu: Allow access to ceph config

[Stefan Bader]

From: Serge Hallyn 

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
---
 examples/apparmor/libvirt-qemu | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
index 7fa512f..fddc93a 100644
--- a/examples/apparmor/libvirt-qemu
+++ b/examples/apparmor/libvirt-qemu
@@ -156,6 +156,9 @@
   /sys/bus/ r,
   /sys/class/ r,
 
+  # for rbd
+  /etc/ceph/ceph.conf r,
+
   # for ppc device-tree access
   @{PROC}/device-tree/ r,
   @{PROC}/device-tree/** r,
-- 
2.7.4

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

[libvirt] [PATCH 05/10] apparmor: Allow pygrub to run on Debian/Ubuntu

[Stefan Bader]

In Debian/Ubuntu the pygrub command is located under
/usr/lib/xen-/bin/pygrub.

Bug-Ubuntu: https://bugs.launchpad.net/bugs/1326003

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
---
 examples/apparmor/usr.sbin.libvirtd | 1 +
 1 file changed, 1 insertion(+)

diff --git a/examples/apparmor/usr.sbin.libvirtd 
b/examples/apparmor/usr.sbin.libvirtd
index 353b039..f43bfd5 100644
--- a/examples/apparmor/usr.sbin.libvirtd
+++ b/examples/apparmor/usr.sbin.libvirtd
@@ -50,6 +50,7 @@
   /{usr/,}lib/udev/scsi_id PUx,
   /usr/{lib,lib64}/xen-common/bin/xen-toolstack PUx,
   /usr/{lib,lib64}/xen/bin/* Ux,
+  /usr/lib/xen-*/bin/pygrub PUx,
 
   # force the use of virt-aa-helper
   audit deny /{usr/,}sbin/apparmor_parser rwxl,
-- 
2.7.4

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

[libvirt] [PATCH 08/10] apparmor, libvirt-qemu: Allow macvtap access

[Stefan Bader]

From: Guilhem Lettron 

Add rule to allow access to /dev/tap* used by macvtap.

Bug-Ubuntu: https://bugs.launchpad.net/bugs/921870

Signed-off-by: Christian Ehrhardt 
Signed-off-by: Stefan Bader 
---
 examples/apparmor/libvirt-qemu | 1 +
 1 file changed, 1 insertion(+)

diff --git a/examples/apparmor/libvirt-qemu b/examples/apparmor/libvirt-qemu
index fddc93a..e2b0dfd 100644
--- a/examples/apparmor/libvirt-qemu
+++ b/examples/apparmor/libvirt-qemu
@@ -17,6 +17,7 @@
   network inet6 stream,
 
   /dev/net/tun rw,
+  /dev/tap* rw,
   /dev/kvm rw,
   /dev/ptmx rw,
   /dev/kqemu rw,
-- 
2.7.4

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

Re: [libvirt] [PATCH 2/2] qemu: don't relabel chardev source file if virtlogd is used

[Daniel P. Berrange]

On Tue, May 23, 2017 at 05:53:46PM +0200, Pavel Hrdina wrote:
> On Tue, May 23, 2017 at 04:24:13PM +0100, Daniel P. Berrange wrote:
> > On Tue, May 23, 2017 at 04:25:01PM +0200, Martin Kletzander wrote:
> > > On Mon, May 15, 2017 at 04:28:35PM +0200, Pavel Hrdina wrote:
> > > > If libvirt uses virtlogd instead of passing the file path directly
> > > > to QEMU we shouldn't relabel the chardev source file, otherwise
> > > > virtlogd will get a permission denied while reloading.
> > > > 
> > > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=143098
> > > > 
> > > 
> > > You missed a digit and I'm too lazy to check 10 bugs for a reproducer ;)
> > > 
> > > > Signed-off-by: Pavel Hrdina 
> > > > ---
> > > > src/conf/domain_conf.c  | 20 
> > > > src/conf/domain_conf.h  |  1 +
> > > > src/qemu/qemu_command.c | 12 
> > > > src/security/security_dac.c |  6 ++
> > > > src/security/security_selinux.c |  6 ++
> > > > 5 files changed, 41 insertions(+), 4 deletions(-)
> > > > 
> > > 
> > > I see you are changing the parser code, but you are not changing the
> > > Relax-NG schema, neither any test.
> > > 
> > > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > > > index aa441fae3c..92f011d3a4 100644
> > > > --- a/src/conf/domain_conf.c
> > > > +++ b/src/conf/domain_conf.c
> > > > @@ -2064,6 +2064,7 @@ 
> > > > virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest,
> > > > }
> > > > 
> > > > dest->type = src->type;
> > > > +dest->skipRelabel = src->skipRelabel;
> > > > 
> > > > return 0;
> > > > }
> > > > @@ -10608,6 +10609,7 @@ 
> > > > virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
> > > > char *append = NULL;
> > > > char *haveTLS = NULL;
> > > > char *tlsFromConfig = NULL;
> > > > +char *skipRelabel = NULL;
> > > > int remaining = 0;
> > > > 
> > > > while (cur != NULL) {
> > > > @@ -10628,6 +10630,8 @@ 
> > > > virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
> > > > case VIR_DOMAIN_CHR_TYPE_UNIX:
> > > > if (!append && def->type == 
> > > > VIR_DOMAIN_CHR_TYPE_FILE)
> > > > append = virXMLPropString(cur, "append");
> > > > +if (!skipRelabel && def->type == 
> > > > VIR_DOMAIN_CHR_TYPE_FILE)
> > > > +skipRelabel = virXMLPropString(cur, 
> > > > "skipRelabel");
> > > 
> > > I'm guessing you want to keep this away from users, right?  You should
> > > not be parsing it from the XML then.  Or you should add a thing there
> > > that the XML supports.  Not just a random attribute.
> > > 
> > > Either keep this data in private structure or even better, just add the
> > > same thing as you would do with:
> > > 
> > >  
> > > 
> > > with all the details of course.  The user can see it, can supply it, old
> > > releases support it, all the stuff is there already.
> > > 
> > > I'm open to suggestions, but NACK to random "wannabe private" attributes.
> > 
> > The use of virtlogd affects many devices in guests. So we should just
> > record at the top level of the QEMU domain status, whether virtlogd
> > was used or not.
> 
> That would be one possibility, however we need to decide what to do in
> one specific case:
> 
> 1. stdio_handler is set to logd and QEMU_CAPS_CHARDEV_FILE_APPEND is
> supported
> 
> 2. start a guest with one serial chardev
> 
> 3. change the stdio_handler to file and restart libvirtd
> 
> 4. hotplug another serial chardev (this is currently broken [1])
> 
> There are to possible solution, we will store the usage of virtlogd
> domain-wide and virtlogd will be used for that domain even if the config
> option would change for the rest of that domain's life.  Or we need to
> store the usage of virtlogd per device and it will always depend on the
> config option.

Having some devices use virtlogd and some devices not use virtlogd
is just a recipe for maximising confusion of developers, users, and
support people alike.

We should record whether we used virtlogd when we first start the
guest, and honour that until QEMU is killed.

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] [libvirt-perl][PATCH 0/7] Implement sparse streams

[Daniel P. Berrange]

On Tue, May 23, 2017 at 05:05:34PM +0200, Michal Privoznik wrote:
> *** BLURB HERE ***
> 
> Michal Privoznik (7):
>   Fix send_all() callback helper
>   Introduce flags to Stream::recv()
>   Introduce Stream::recvHole() and Stream::sendHole()
>   Introduce Stream::sparse_recv_all()
>   Introduce Stream::sparse_send_all()
>   Register VOL_DOWNLOAD_SPARSE_STREAM & VOL_UPLOAD_SPARSE_STREAM
> constants
>   examples: Introduce vol-sparse.pl
> 
>  Changes|   9 ++
>  Virt.xs| 241 
> -
>  examples/vol-sparse.pl | 142 ++
>  lib/Sys/Virt/StorageVol.pm |  30 +-
>  lib/Sys/Virt/Stream.pm |  70 -
>  t/030-api-coverage.t   |   3 +
>  6 files changed, 483 insertions(+), 12 deletions(-)
>  create mode 100755 examples/vol-sparse.pl

ACK with the couple of renames suggested


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] [libvirt-perl][PATCH 5/7] Introduce Stream::sparse_send_all()

[Daniel P. Berrange]

On Tue, May 23, 2017 at 05:05:39PM +0200, Michal Privoznik wrote:
> Signed-off-by: Michal Privoznik 
> ---
>  Changes|   1 +
>  Virt.xs| 124 
> +
>  lib/Sys/Virt/Stream.pm |  19 
>  t/030-api-coverage.t   |   2 +
>  4 files changed, 146 insertions(+)
> 
> diff --git a/Changes b/Changes
> index 4c1e071..989b795 100644
> --- a/Changes
> +++ b/Changes
> @@ -8,6 +8,7 @@ Revision history for perl module Sys::Virt
> register RECV_STOP_AT_HOLE constant
>   - Introduce Stream::recvHole() and Stream::sendHole()
>   - Introduce Stream::sparse_recv_all()
> + - Introduce Stream::sparse_send_all()
>  
>  3.3.0 2017-05-08
>  
> diff --git a/Virt.xs b/Virt.xs
> index 002bd6a..fc282b8 100644
> --- a/Virt.xs
> +++ b/Virt.xs
> @@ -1960,6 +1960,100 @@ _stream_send_all_source(virStreamPtr st,
>  }
>  
>  
> +static int
> +_stream_sparse_send_all_holeHandler(virStreamPtr st,
> +int *inData,
> +long long *length,
> +void *opaque)

Use _handler, rather than Handler - and the same elsewhere in this
patch.

> +{
> +AV *av = opaque;
> +SV **self;
> +SV **holeHandler;
> +SV *inDataSV;
> +SV *lengthSV;
> +int count;
> +int ret;
> +dSP;
> +
> +self = av_fetch(av, 0, 0);
> +holeHandler = av_fetch(av, 2, 0);
> +
> +SvREFCNT_inc(*self);
> +
> +ENTER;
> +SAVETMPS;
> +
> +PUSHMARK(SP);
> +XPUSHs(*self);
> +PUTBACK;
> +
> +count = call_sv((SV*)*holeHandler, G_ARRAY);
> +
> +SPAGAIN;
> +
> +if (count == 2) {
> +/* @holeHandler returns (inData, length), but on a stack.
> + * Therefore the order is reversed. */
> +lengthSV = POPs;
> +inDataSV = POPs;
> +*inData = virt_SvIVll(inDataSV);
> +*length = virt_SvIVll(lengthSV);
> +ret = 0;
> +} else {
> +ret = -1;
> +}
> +
> +PUTBACK;
> +FREETMPS;
> +LEAVE;
> +
> +return ret;
> +}
> +
> +
> +static int
> +_stream_sparse_send_all_skipHandler(virStreamPtr st,
> +long long length,
> +void *opaque)
> +{
> +AV *av = opaque;
> +SV **self;
> +SV **skipHandler;
> +int rv;
> +int ret;
> +dSP;
> +
> +self = av_fetch(av, 0, 0);
> +skipHandler = av_fetch(av, 3, 0);
> +
> +SvREFCNT_inc(*self);
> +
> +ENTER;
> +SAVETMPS;
> +
> +PUSHMARK(SP);
> +XPUSHs(*self);
> +XPUSHs(sv_2mortal(virt_newSVll(length)));
> +PUTBACK;
> +
> +rv = call_sv((SV*)*skipHandler, G_SCALAR);
> +
> +SPAGAIN;
> +
> +if (rv == 1) {
> +ret = POPi;
> +} else {
> +ret = -1;
> +}
> +
> +PUTBACK;
> +FREETMPS;
> +LEAVE;
> +
> +return ret;
> +}
> +
> +
>  static int
>  _stream_recv_all_sink(virStreamPtr st,
>const char *data,
> @@ -8041,6 +8135,36 @@ sparse_recv_all(stref, handler, holeHandler)
>  
>SvREFCNT_dec(opaque);
>  
> +void
> +sparse_send_all(stref, handler, holeHandler, skipHandler)
> +  SV *stref;
> +  SV *handler;
> +  SV *holeHandler;
> +  SV *skipHandler;
> + PREINIT:
> +  AV *opaque;
> +  virStreamPtr st;
> +CODE:
> +  st = (virStreamPtr)SvIV((SV*)SvRV(stref));
> +
> +  opaque = newAV();
> +  SvREFCNT_inc(stref);
> +  SvREFCNT_inc(handler);
> +  SvREFCNT_inc(holeHandler);
> +  SvREFCNT_inc(skipHandler);
> +  av_push(opaque, stref);
> +  av_push(opaque, handler);
> +  av_push(opaque, holeHandler);
> +  av_push(opaque, skipHandler);
> +
> +  if (virStreamSparseSendAll(st,
> + _stream_send_all_source,
> + _stream_sparse_send_all_holeHandler,
> + _stream_sparse_send_all_skipHandler,
> + opaque) < 0)
> +  _croak_error();
> +
> +  SvREFCNT_dec(opaque);
>  
>  void
>  add_callback(stref, events, cb)
> diff --git a/lib/Sys/Virt/Stream.pm b/lib/Sys/Virt/Stream.pm
> index c32b957..e3b25a5 100644
> --- a/lib/Sys/Virt/Stream.pm
> +++ b/lib/Sys/Virt/Stream.pm
> @@ -144,6 +144,25 @@ bytes). The C<$holeHandler> is expected to return a 
> non-negative
>  number on success (usually 0) and a negative number (usually -1)
>  otherwise.
>  
> +=item $st->sparse_send_all($handler, $holeHandler, $skipHandler)
> +
> +Send all data produced by C<$handler> to the stream.  The
> +C<$handler> parameter must be a function which expects three
> +arguments, the C<$st> stream object, a scalar which must be
> +filled with data and a maximum data byte count desired.  The
> +function should return the number of bytes filled, 0 on end of
> +file, or -1 upon error. The second argument C<$holeHandler> is a
> +function expecting just one argument C<$st> and returning an
> 

Re: [libvirt] [PATCH 2/2] qemu: don't relabel chardev source file if virtlogd is used

[Pavel Hrdina]

On Tue, May 23, 2017 at 04:24:13PM +0100, Daniel P. Berrange wrote:
> On Tue, May 23, 2017 at 04:25:01PM +0200, Martin Kletzander wrote:
> > On Mon, May 15, 2017 at 04:28:35PM +0200, Pavel Hrdina wrote:
> > > If libvirt uses virtlogd instead of passing the file path directly
> > > to QEMU we shouldn't relabel the chardev source file, otherwise
> > > virtlogd will get a permission denied while reloading.
> > > 
> > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=143098
> > > 
> > 
> > You missed a digit and I'm too lazy to check 10 bugs for a reproducer ;)
> > 
> > > Signed-off-by: Pavel Hrdina 
> > > ---
> > > src/conf/domain_conf.c  | 20 
> > > src/conf/domain_conf.h  |  1 +
> > > src/qemu/qemu_command.c | 12 
> > > src/security/security_dac.c |  6 ++
> > > src/security/security_selinux.c |  6 ++
> > > 5 files changed, 41 insertions(+), 4 deletions(-)
> > > 
> > 
> > I see you are changing the parser code, but you are not changing the
> > Relax-NG schema, neither any test.
> > 
> > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > > index aa441fae3c..92f011d3a4 100644
> > > --- a/src/conf/domain_conf.c
> > > +++ b/src/conf/domain_conf.c
> > > @@ -2064,6 +2064,7 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr 
> > > dest,
> > > }
> > > 
> > > dest->type = src->type;
> > > +dest->skipRelabel = src->skipRelabel;
> > > 
> > > return 0;
> > > }
> > > @@ -10608,6 +10609,7 @@ 
> > > virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
> > > char *append = NULL;
> > > char *haveTLS = NULL;
> > > char *tlsFromConfig = NULL;
> > > +char *skipRelabel = NULL;
> > > int remaining = 0;
> > > 
> > > while (cur != NULL) {
> > > @@ -10628,6 +10630,8 @@ 
> > > virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
> > > case VIR_DOMAIN_CHR_TYPE_UNIX:
> > > if (!append && def->type == VIR_DOMAIN_CHR_TYPE_FILE)
> > > append = virXMLPropString(cur, "append");
> > > +if (!skipRelabel && def->type == 
> > > VIR_DOMAIN_CHR_TYPE_FILE)
> > > +skipRelabel = virXMLPropString(cur, 
> > > "skipRelabel");
> > 
> > I'm guessing you want to keep this away from users, right?  You should
> > not be parsing it from the XML then.  Or you should add a thing there
> > that the XML supports.  Not just a random attribute.
> > 
> > Either keep this data in private structure or even better, just add the
> > same thing as you would do with:
> > 
> >  
> > 
> > with all the details of course.  The user can see it, can supply it, old
> > releases support it, all the stuff is there already.
> > 
> > I'm open to suggestions, but NACK to random "wannabe private" attributes.
> 
> The use of virtlogd affects many devices in guests. So we should just
> record at the top level of the QEMU domain status, whether virtlogd
> was used or not.

That would be one possibility, however we need to decide what to do in
one specific case:

1. stdio_handler is set to logd and QEMU_CAPS_CHARDEV_FILE_APPEND is
supported

2. start a guest with one serial chardev

3. change the stdio_handler to file and restart libvirtd

4. hotplug another serial chardev (this is currently broken [1])

There are to possible solution, we will store the usage of virtlogd
domain-wide and virtlogd will be used for that domain even if the config
option would change for the rest of that domain's life.  Or we need to
store the usage of virtlogd per device and it will always depend on the
config option.

[1] The hotplug of char devices doesn't use virtlogd at all and we don't
relabel the files to be accessible by QEMU.

Pavel


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

Re: [libvirt] [libvirt-perl][PATCH 3/7] Introduce Stream::recvHole() and Stream::sendHole()

[Daniel P. Berrange]

On Tue, May 23, 2017 at 05:05:37PM +0200, Michal Privoznik wrote:
> Signed-off-by: Michal Privoznik 
> ---
>  Changes|  1 +
>  Virt.xs| 28 
>  lib/Sys/Virt/Stream.pm | 17 +
>  3 files changed, 46 insertions(+)
> 
> diff --git a/Changes b/Changes
> index b4a493c..c92c271 100644
> --- a/Changes
> +++ b/Changes
> @@ -6,6 +6,7 @@ Revision history for perl module Sys::Virt
>   - Fix send_all() callback helper
>   - Introduce flags to Stream::recv() and
> register RECV_STOP_AT_HOLE constant
> + - Introduce Stream::recvHole() and Stream::sendHole()
>  
>  3.3.0 2017-05-08
>  
> diff --git a/Virt.xs b/Virt.xs
> index 498e711..d112708 100644
> --- a/Virt.xs
> +++ b/Virt.xs
> @@ -7900,6 +7900,34 @@ recv(st, data, nbytes, flags=0)
>RETVAL
>  
>  
> +SV *
> +recvHole(st, flags=0)
> +  virStreamPtr st;
> +  unsigned int flags;
> +  PREINIT:
> +  long long length;
> +CODE:
> +  if (virStreamRecvHole(st, , flags) < 0)
> +  _croak_error();
> +
> +  RETVAL = virt_newSVll(length);
> +  OUTPUT:
> +  RETVAL
> +
> +
> +void
> +sendHole(st, lengthSV, flags=0)
> +  virStreamPtr st;
> +  SV *lengthSV;
> +  unsigned int flags;
> + PREINIT:
> +  long long length;
> +  PPCODE:
> +  length = virt_SvIVll(lengthSV);
> +  if (virStreamSendHole(st, length, flags) < 0)
> +  _croak_error();
> +
> +
>  void
>  send_all(stref, handler)
>SV *stref;
> diff --git a/lib/Sys/Virt/Stream.pm b/lib/Sys/Virt/Stream.pm
> index 069895e..5984fb6 100644
> --- a/lib/Sys/Virt/Stream.pm
> +++ b/lib/Sys/Virt/Stream.pm
> @@ -93,6 +93,23 @@ Send upto C<$nbytes> worth of data, copying from C<$data>.
>  Returns the number of bytes sent, or -2 if I/O would block,
>  or -1 on error.
>  
> +=item $rv = $st->recvHole($flags=0)
> +
> +Determine the amount of the empty space (in bytes) to be created
> +in a stream's target file when uploading or downloading sparsely
> +populated files. This is the counterpart to C. The
> +optional C<$flags> parameter is currently unused and defaults to
> +zero if omitted.
> +
> +=item $st->sendHole($length, $flags=0)
> +
> +Rather than transmitting empty file space, this method directs
> +the stream target to create C<$length> bytes of empty space.
> +This method would be used when uploading or downloading sparsely
> +populated files to avoid the needless copy of empty file space.
> +The optional C<$flags> parameter is currently unused and defaults
> +to zero if omitted.

These methods should use '_' rather than camelCase in naming.


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] qemu: don't relabel chardev source file if virtlogd is used

[Pavel Hrdina]

On Tue, May 23, 2017 at 04:25:01PM +0200, Martin Kletzander wrote:
> On Mon, May 15, 2017 at 04:28:35PM +0200, Pavel Hrdina wrote:
> >If libvirt uses virtlogd instead of passing the file path directly
> >to QEMU we shouldn't relabel the chardev source file, otherwise
> >virtlogd will get a permission denied while reloading.
> >
> >Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=143098
> >
> 
> You missed a digit and I'm too lazy to check 10 bugs for a reproducer ;)

Yep, there should by additional 8 :)

> >Signed-off-by: Pavel Hrdina 
> >---
> > src/conf/domain_conf.c  | 20 
> > src/conf/domain_conf.h  |  1 +
> > src/qemu/qemu_command.c | 12 
> > src/security/security_dac.c |  6 ++
> > src/security/security_selinux.c |  6 ++
> > 5 files changed, 41 insertions(+), 4 deletions(-)
> >
> 
> I see you are changing the parser code, but you are not changing the
> Relax-NG schema, neither any test.
> 
> >diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> >index aa441fae3c..92f011d3a4 100644
> >--- a/src/conf/domain_conf.c
> >+++ b/src/conf/domain_conf.c
> >@@ -2064,6 +2064,7 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr 
> >dest,
> > }
> >
> > dest->type = src->type;
> >+dest->skipRelabel = src->skipRelabel;
> >
> > return 0;
> > }
> >@@ -10608,6 +10609,7 @@ 
> >virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
> > char *append = NULL;
> > char *haveTLS = NULL;
> > char *tlsFromConfig = NULL;
> >+char *skipRelabel = NULL;
> > int remaining = 0;
> >
> > while (cur != NULL) {
> >@@ -10628,6 +10630,8 @@ 
> >virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
> > case VIR_DOMAIN_CHR_TYPE_UNIX:
> > if (!append && def->type == VIR_DOMAIN_CHR_TYPE_FILE)
> > append = virXMLPropString(cur, "append");
> >+if (!skipRelabel && def->type == 
> >VIR_DOMAIN_CHR_TYPE_FILE)
> >+skipRelabel = virXMLPropString(cur, "skipRelabel");
> 
> I'm guessing you want to keep this away from users, right?  You should
> not be parsing it from the XML then.  Or you should add a thing there
> that the XML supports.  Not just a random attribute.

We need to store it in the status XML so we don't lose the information
while restarting libvirtd.  And I want to keep it hidden from users
because they shouldn't care about it.  The decision whether we will use
virtlogd as stdio handler is made by checking the qemu.conf for
"stdio_handler" and whether QEMU supports appending to files.

> Either keep this data in private structure or even better, just add the

What do you mean by private structure?

> same thing as you would do with:
> 
>   
>
> with all the details of course.  The user can see it, can supply it, old
> releases support it, all the stuff is there already.

This for user to specify it, not to store our internal private
information that should survive restarting libvirtd.

> I'm open to suggestions, but NACK to random "wannabe private" attributes.

The only reason why I used the new private attribute is that we already
do it for graphics device (fromConfig and autoGenerated) and for
TCP character device (tlsFromConfig).

I can store it in the status part of the status XML outside of the
domain part, but that would require mapping it to the actual character
device.

Pavel


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

Re: [libvirt] [PATCH v2] lib: Add note that bulk stats API queries may overrun RPC buffers

[Peter Krempa]

On Tue, May 23, 2017 at 17:19:53 +0200, Martin Kletzander wrote:
> On Tue, May 23, 2017 at 05:07:40PM +0200, Michal Privoznik wrote:
> > On 05/23/2017 04:35 PM, Martin Kletzander wrote:
> > > On Tue, May 23, 2017 at 04:23:30PM +0200, Peter Krempa wrote:

[...]

> > > > + * Note that this API is prone to exceeding maximum RPC if querying
> > > > too many VMs
> > > > + * with lots of statistics. It's suggested to query in batches of
> > > > 10VMs, which
> > > > + * should be good enough for VMs with 3000 disks + networks.
> > > > + *
> > > 
> > > Coming to think about it... Why don't we just batch this ourselves under
> > > the hood and just return the merged result?
> > 
> > Because:
> > 
> > https://www.redhat.com/archives/libvir-list/2017-May/msg00088.html
> > 
> 
> Not on the RPC level, the API would just be syntax sugar to
> virDomainListGetStats() if a flag was passed
> (e.g. 
> VIR_DOMAIN_GET_ALL_STATS_I_DONT_REALLY_CARE_IF_THIS_IS_DONE_IN_ONE_LIBVIRT_CALL)

Also compared to a full fragmentation of the returned data, this would
result into a worst-case-scenario memory usage of MAX_SIZE *
NVMS_QUERIED_IN_ORIGINAL_CALL, when compared to an unbounded memory use
of the full fragmentation approach.


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

Re: [libvirt] [PATCH v2] lib: Add note that bulk stats API queries may overrun RPC buffers

[Michal Privoznik]

On 05/23/2017 05:19 PM, Martin Kletzander wrote:
> On Tue, May 23, 2017 at 05:07:40PM +0200, Michal Privoznik wrote:
>> On 05/23/2017 04:35 PM, Martin Kletzander wrote:
>>> On Tue, May 23, 2017 at 04:23:30PM +0200, Peter Krempa wrote:
 Hint that the users should limit the number of VMs queried in the bulk
 stats API.
 ---
 v2:
 - added a suggestion of the number of queried VMs (valid after bump to
 32M message)

 src/libvirt-domain.c | 8 
 1 file changed, 8 insertions(+)

 diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
 index 310b91b37..aef061943 100644
 --- a/src/libvirt-domain.c
 +++ b/src/libvirt-domain.c
 @@ -11315,6 +11315,10 @@ virConnectGetDomainCapabilities(virConnectPtr
 conn,
  * VIR_CONNECT_GET_ALL_DOMAINS_STATS_SHUTOFF and/or
  * VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER for all other states.
  *
 + * Note that this API is prone to exceeding maximum RPC message size
 on hosts
 + * with lots of VMs so it's suggested to use virDomainListGetStats
 with a
 + * reasonable list of VMs as the argument.
 + *
  * Returns the count of returned statistics structures on success, -1
 on error.
  * The requested data are returned in the @retStats parameter. The
 returned
  * array should be freed by the caller. See
 virDomainStatsRecordListFree.
 @@ -11386,6 +11390,10 @@ virConnectGetAllDomainStats(virConnectPtr
 conn,
  * Note that any of the domain list filtering flags in @flags may be
 rejected
  * by this function.
  *
 + * Note that this API is prone to exceeding maximum RPC if querying
 too many VMs
 + * with lots of statistics. It's suggested to query in batches of
 10VMs, which
 + * should be good enough for VMs with 3000 disks + networks.
 + *
>>>
>>> Coming to think about it... Why don't we just batch this ourselves under
>>> the hood and just return the merged result?
>>
>> Because:
>>
>> https://www.redhat.com/archives/libvir-list/2017-May/msg00088.html
>>
> 
> Not on the RPC level, the API would just be syntax sugar to
> virDomainListGetStats() if a flag was passed
> (e.g.
> VIR_DOMAIN_GET_ALL_STATS_I_DONT_REALLY_CARE_IF_THIS_IS_DONE_IN_ONE_LIBVIRT_CALL)

We can't guarantee atomicity of such call, so I don't see a reason for
us to implement it. On the other hand, it's us who knows the limits for
RPC, not users. So they might have some troubles writing it. But then
again, we would have some troubles too because we might be talking to an
older server with lower limits thus we'd need some guessing too.

Also, the flag would expose what we want to hide - RPC layer. It should
be no concern for users how we pass data around.

Michal

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

Re: [libvirt] [PATCH 2/2] qemu: don't relabel chardev source file if virtlogd is used

[Martin Kletzander]

On Tue, May 23, 2017 at 05:13:17PM +0200, Erik Skultety wrote:

> while (cur != NULL) {
> @@ -10628,6 +10630,8 @@ 
virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
> case VIR_DOMAIN_CHR_TYPE_UNIX:
> if (!append && def->type == VIR_DOMAIN_CHR_TYPE_FILE)
> append = virXMLPropString(cur, "append");
> +if (!skipRelabel && def->type == 
VIR_DOMAIN_CHR_TYPE_FILE)
> +skipRelabel = virXMLPropString(cur, "skipRelabel");

I'm guessing you want to keep this away from users, right?  You should
not be parsing it from the XML then.  Or you should add a thing there
that the XML supports.  Not just a random attribute.

Either keep this data in private structure or even better, just add the
same thing as you would do with:

 


What sense would it make in this case? There is only one option when we're
running virtlogd and that is no label. If they supply relabel='yes', it won't
work with virtlogd unless they change it back to 'no'. So, you've got an option
the functionality of which depends on a service running, which is by default on
by the way.



If that's so clear, why put it in the XML?  Why not just set it in the
private structure then?  Either there is a reason for it to be variable
or not.  If there is not, then we're effectively talking about something
like  attribute.  There is no need for
such thing in that case.

Sure, I might be wrong.  In that case, this commit needs more explanation.


Since I don't want to be someone who just criticizes other people's ideas
without adding anything to the discussion, I was thinking about re-using the
qemu config parameter stdioLogD. But that solution is also wrong as it turned
out. I haven't come up with anything better yet, though. So, having something in
the status XML seems like a plausible approach to me.

Erik



with all the details of course.  The user can see it, can supply it, old
releases support it, all the stuff is there already.

I'm open to suggestions, but NACK to random "wannabe private" attributes.





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


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


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

Re: [libvirt] [PATCH 2/2] qemu: don't relabel chardev source file if virtlogd is used

[Daniel P. Berrange]

On Tue, May 23, 2017 at 04:25:01PM +0200, Martin Kletzander wrote:
> On Mon, May 15, 2017 at 04:28:35PM +0200, Pavel Hrdina wrote:
> > If libvirt uses virtlogd instead of passing the file path directly
> > to QEMU we shouldn't relabel the chardev source file, otherwise
> > virtlogd will get a permission denied while reloading.
> > 
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=143098
> > 
> 
> You missed a digit and I'm too lazy to check 10 bugs for a reproducer ;)
> 
> > Signed-off-by: Pavel Hrdina 
> > ---
> > src/conf/domain_conf.c  | 20 
> > src/conf/domain_conf.h  |  1 +
> > src/qemu/qemu_command.c | 12 
> > src/security/security_dac.c |  6 ++
> > src/security/security_selinux.c |  6 ++
> > 5 files changed, 41 insertions(+), 4 deletions(-)
> > 
> 
> I see you are changing the parser code, but you are not changing the
> Relax-NG schema, neither any test.
> 
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index aa441fae3c..92f011d3a4 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -2064,6 +2064,7 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr 
> > dest,
> > }
> > 
> > dest->type = src->type;
> > +dest->skipRelabel = src->skipRelabel;
> > 
> > return 0;
> > }
> > @@ -10608,6 +10609,7 @@ 
> > virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
> > char *append = NULL;
> > char *haveTLS = NULL;
> > char *tlsFromConfig = NULL;
> > +char *skipRelabel = NULL;
> > int remaining = 0;
> > 
> > while (cur != NULL) {
> > @@ -10628,6 +10630,8 @@ 
> > virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
> > case VIR_DOMAIN_CHR_TYPE_UNIX:
> > if (!append && def->type == VIR_DOMAIN_CHR_TYPE_FILE)
> > append = virXMLPropString(cur, "append");
> > +if (!skipRelabel && def->type == 
> > VIR_DOMAIN_CHR_TYPE_FILE)
> > +skipRelabel = virXMLPropString(cur, "skipRelabel");
> 
> I'm guessing you want to keep this away from users, right?  You should
> not be parsing it from the XML then.  Or you should add a thing there
> that the XML supports.  Not just a random attribute.
> 
> Either keep this data in private structure or even better, just add the
> same thing as you would do with:
> 
>  
> 
> with all the details of course.  The user can see it, can supply it, old
> releases support it, all the stuff is there already.
> 
> I'm open to suggestions, but NACK to random "wannabe private" attributes.

The use of virtlogd affects many devices in guests. So we should just
record at the top level of the QEMU domain status, whether virtlogd
was used or not.


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 v2] lib: Add note that bulk stats API queries may overrun RPC buffers

[Martin Kletzander]

On Tue, May 23, 2017 at 05:07:40PM +0200, Michal Privoznik wrote:

On 05/23/2017 04:35 PM, Martin Kletzander wrote:

On Tue, May 23, 2017 at 04:23:30PM +0200, Peter Krempa wrote:

Hint that the users should limit the number of VMs queried in the bulk
stats API.
---
v2:
- added a suggestion of the number of queried VMs (valid after bump to
32M message)

src/libvirt-domain.c | 8 
1 file changed, 8 insertions(+)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 310b91b37..aef061943 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11315,6 +11315,10 @@ virConnectGetDomainCapabilities(virConnectPtr
conn,
 * VIR_CONNECT_GET_ALL_DOMAINS_STATS_SHUTOFF and/or
 * VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER for all other states.
 *
+ * Note that this API is prone to exceeding maximum RPC message size
on hosts
+ * with lots of VMs so it's suggested to use virDomainListGetStats
with a
+ * reasonable list of VMs as the argument.
+ *
 * Returns the count of returned statistics structures on success, -1
on error.
 * The requested data are returned in the @retStats parameter. The
returned
 * array should be freed by the caller. See virDomainStatsRecordListFree.
@@ -11386,6 +11390,10 @@ virConnectGetAllDomainStats(virConnectPtr conn,
 * Note that any of the domain list filtering flags in @flags may be
rejected
 * by this function.
 *
+ * Note that this API is prone to exceeding maximum RPC if querying
too many VMs
+ * with lots of statistics. It's suggested to query in batches of
10VMs, which
+ * should be good enough for VMs with 3000 disks + networks.
+ *


Coming to think about it... Why don't we just batch this ourselves under
the hood and just return the merged result?


Because:

https://www.redhat.com/archives/libvir-list/2017-May/msg00088.html



Not on the RPC level, the API would just be syntax sugar to
virDomainListGetStats() if a flag was passed
(e.g. 
VIR_DOMAIN_GET_ALL_STATS_I_DONT_REALLY_CARE_IF_THIS_IS_DONE_IN_ONE_LIBVIRT_CALL)


Michal

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


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

Re: [libvirt] [libvirt-python][PATCH v2 3/4] virStream: Introduce virStreamSparse{Recv, Send}All

[Martin Kletzander]

On Tue, May 23, 2017 at 04:26:10PM +0200, Michal Privoznik wrote:

Yet again, our parser is not capable of generating proper
wrapper. To be fair, this one wold be really tough anyway.

Signed-off-by: Michal Privoznik 
---
generator.py  |   2 +
libvirt-override-virStream.py | 107 ++
sanitytest.py |   6 ++-
3 files changed, 113 insertions(+), 2 deletions(-)

diff --git a/generator.py b/generator.py
index 0e07fc8..93d1dc3 100755
--- a/generator.py
+++ b/generator.py
@@ -546,6 +546,8 @@ skip_function = (
'virStreamRecvHole', # overridden in libvirt-override-virStream.py
'virStreamSendHole', # overridden in libvirt-override-virStream.py
'virStreamRecvFlags', # overridden in libvirt-override-virStream.py
+'virStreamSparseRecvAll', # overridden in libvirt-override-virStream.py
+'virStreamSparseSendAll', # overridden in libvirt-override-virStream.py

'virConnectUnregisterCloseCallback', # overridden in virConnect.py
'virConnectRegisterCloseCallback', # overridden in virConnect.py
diff --git a/libvirt-override-virStream.py b/libvirt-override-virStream.py
index 66d2bf6..0ab7815 100644
--- a/libvirt-override-virStream.py
+++ b/libvirt-override-virStream.py
@@ -164,3 +164,110 @@
ret = libvirtmod.virStreamRecvFlags(self._o, nbytes, flags)
if ret is None: raise libvirtError ('virStreamRecvFlags() failed')
return ret
+
+def sparseRecvAll(self, handler, holeHandler, opaque):
+"""Receive the entire data stream, sending the data to
+the requested data sink handler and calling the skip
+holeHandler to generate holes for sparse stream targets.
+This is simply a convenient alternative to recvFlags, for
+apps that do blocking-I/O and want to preserve sparseness.
+
+Hypothetical callbacks can look like this:
+
+def handler(stream, # virStream instance
+buf,# string containing received data
+opaque): # extra data passed to sparseRecvAll as opaque
+fd = opaque
+return os.write(fd, buf)
+
+def holeHandler(stream, # virStream instance
+length, # number of bytes to skip
+opaque): # extra data passed to sparseRecvAll as 
opaque
+fd = opaque
+cur = os.lseek(fd, length, os.SEEK_CUR)
+return os.ftruncate(fd, cur) # take this extra step to
+ # actually allocate the hole
+"""
+while True:
+want = 64 * 1024
+got = self.recvFlags(want, VIR_STREAM_RECV_STOP_AT_HOLE)
+if got == -2:
+raise libvirtError("cannot use sparseRecvAll with "
+   "nonblocking stream")
+if got == -3:
+length = self.recvHole()
+if length is None:
+self.abort()
+raise RuntimeError("recvHole handler failed")
+ret = holeHandler(self, length, opaque)
+if type(ret) is int and ret < 0:
+self.abort()
+raise RuntimeError("holeHandler handler returned %d" % ret)
+continue
+
+if len(got) == 0:
+break
+
+try:
+ret = handler(self, got, opaque)
+if type(ret) is int and ret < 0:
+raise RuntimeError("sparseRecvAll handler returned %d" % 
ret)
+except Exception as e:


What exception are you trying to catch here?  Exception means something
went wrong.


+self.abort()
+raise e
+
+def sparseSendAll(self, handler, holeHandler, skipHandler, opaque):
+"""Send the entire data stream, reading the data from the
+requested data source. This is simply a convenient
+alternative to virStreamSend, for apps that do
+blocking-I/O and want to preserve sparseness.
+
+Hypothetical callbacks can look like this:
+
+def handler(stream, # virStream instance
+nbytes, # int amt of data to read
+opaque): # extra data passed to sparseSendAll as opaque
+fd = opaque
+return os.read(fd, nbytes)
+
+def holeHandler(stream, # virStream instance
+opaque): # extra data passed to sparseSendAll as 
opaque
+fd = opaque
+cur = os.lseek(fd, 0, os.SEEK_CUR)
+# ... find out current section and its boundaries
+# and set inData = True/False and sectionLen correspondingly
+os.lseek(fd, cur, os.SEEK_SET)
+return [inData, sectionLen]
+
+def skipHandler(stream, # virStream instance
+  

Re: [libvirt] [PATCH 2/2] qemu: don't relabel chardev source file if virtlogd is used

[Erik Skultety]

> > while (cur != NULL) {
> > @@ -10628,6 +10630,8 @@ 
> > virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
> > case VIR_DOMAIN_CHR_TYPE_UNIX:
> > if (!append && def->type == VIR_DOMAIN_CHR_TYPE_FILE)
> > append = virXMLPropString(cur, "append");
> > +if (!skipRelabel && def->type == 
> > VIR_DOMAIN_CHR_TYPE_FILE)
> > +skipRelabel = virXMLPropString(cur, "skipRelabel");
>
> I'm guessing you want to keep this away from users, right?  You should
> not be parsing it from the XML then.  Or you should add a thing there
> that the XML supports.  Not just a random attribute.
>
> Either keep this data in private structure or even better, just add the
> same thing as you would do with:
>
>  

What sense would it make in this case? There is only one option when we're
running virtlogd and that is no label. If they supply relabel='yes', it won't
work with virtlogd unless they change it back to 'no'. So, you've got an option
the functionality of which depends on a service running, which is by default on
by the way.

Since I don't want to be someone who just criticizes other people's ideas
without adding anything to the discussion, I was thinking about re-using the
qemu config parameter stdioLogD. But that solution is also wrong as it turned
out. I haven't come up with anything better yet, though. So, having something in
the status XML seems like a plausible approach to me.

Erik

>
> with all the details of course.  The user can see it, can supply it, old
> releases support it, all the stuff is there already.
>
> I'm open to suggestions, but NACK to random "wannabe private" attributes.



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

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

Re: [libvirt] [PATCH v2] lib: Add note that bulk stats API queries may overrun RPC buffers

[Michal Privoznik]

On 05/23/2017 04:35 PM, Martin Kletzander wrote:
> On Tue, May 23, 2017 at 04:23:30PM +0200, Peter Krempa wrote:
>> Hint that the users should limit the number of VMs queried in the bulk
>> stats API.
>> ---
>> v2:
>> - added a suggestion of the number of queried VMs (valid after bump to
>> 32M message)
>>
>> src/libvirt-domain.c | 8 
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
>> index 310b91b37..aef061943 100644
>> --- a/src/libvirt-domain.c
>> +++ b/src/libvirt-domain.c
>> @@ -11315,6 +11315,10 @@ virConnectGetDomainCapabilities(virConnectPtr
>> conn,
>>  * VIR_CONNECT_GET_ALL_DOMAINS_STATS_SHUTOFF and/or
>>  * VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER for all other states.
>>  *
>> + * Note that this API is prone to exceeding maximum RPC message size
>> on hosts
>> + * with lots of VMs so it's suggested to use virDomainListGetStats
>> with a
>> + * reasonable list of VMs as the argument.
>> + *
>>  * Returns the count of returned statistics structures on success, -1
>> on error.
>>  * The requested data are returned in the @retStats parameter. The
>> returned
>>  * array should be freed by the caller. See virDomainStatsRecordListFree.
>> @@ -11386,6 +11390,10 @@ virConnectGetAllDomainStats(virConnectPtr conn,
>>  * Note that any of the domain list filtering flags in @flags may be
>> rejected
>>  * by this function.
>>  *
>> + * Note that this API is prone to exceeding maximum RPC if querying
>> too many VMs
>> + * with lots of statistics. It's suggested to query in batches of
>> 10VMs, which
>> + * should be good enough for VMs with 3000 disks + networks.
>> + *
> 
> Coming to think about it... Why don't we just batch this ourselves under
> the hood and just return the merged result?

Because:

https://www.redhat.com/archives/libvir-list/2017-May/msg00088.html

Michal

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

[libvirt] [libvirt-perl][PATCH 4/7] Introduce Stream::sparse_recv_all()

[Michal Privoznik]

Signed-off-by: Michal Privoznik 
---
 Changes|  1 +
 Virt.xs| 70 ++
 lib/Sys/Virt/Stream.pm | 14 ++
 t/030-api-coverage.t   |  1 +
 4 files changed, 86 insertions(+)

diff --git a/Changes b/Changes
index c92c271..4c1e071 100644
--- a/Changes
+++ b/Changes
@@ -7,6 +7,7 @@ Revision history for perl module Sys::Virt
  - Introduce flags to Stream::recv() and
register RECV_STOP_AT_HOLE constant
  - Introduce Stream::recvHole() and Stream::sendHole()
+ - Introduce Stream::sparse_recv_all()
 
 3.3.0 2017-05-08
 
diff --git a/Virt.xs b/Virt.xs
index d112708..002bd6a 100644
--- a/Virt.xs
+++ b/Virt.xs
@@ -2008,6 +2008,48 @@ _stream_recv_all_sink(virStreamPtr st,
 }
 
 
+static int
+_stream_sparse_recv_holeHandler(virStreamPtr st,
+long long offset,
+void *opaque)
+{
+AV *av = opaque;
+SV **self;
+SV **holeHandler;
+int rv;
+int ret;
+dSP;
+
+self = av_fetch(av, 0, 0);
+holeHandler = av_fetch(av, 2, 0);
+
+SvREFCNT_inc(*self);
+
+ENTER;
+SAVETMPS;
+
+PUSHMARK(SP);
+XPUSHs(*self);
+XPUSHs(sv_2mortal(virt_newSVll(offset)));
+PUTBACK;
+
+rv = call_sv((SV*)*holeHandler, G_SCALAR);
+
+SPAGAIN;
+
+if (rv == 1) {
+ret = POPi;
+} else {
+ret = -1;
+}
+
+FREETMPS;
+LEAVE;
+
+return ret;
+}
+
+
 MODULE = Sys::Virt  PACKAGE = Sys::Virt
 
 PROTOTYPES: ENABLE
@@ -7972,6 +8014,34 @@ recv_all(stref, handler)
   SvREFCNT_dec(opaque);
 
 
+void
+sparse_recv_all(stref, handler, holeHandler)
+  SV *stref;
+  SV *handler;
+  SV *holeHandler;
+ PREINIT:
+  AV *opaque;
+  virStreamPtr st;
+CODE:
+  st = (virStreamPtr)SvIV((SV*)SvRV(stref));
+
+  opaque = newAV();
+  SvREFCNT_inc(stref);
+  SvREFCNT_inc(handler);
+  SvREFCNT_inc(holeHandler);
+  av_push(opaque, stref);
+  av_push(opaque, handler);
+  av_push(opaque, holeHandler);
+
+  if (virStreamSparseRecvAll(st,
+ _stream_recv_all_sink,
+ _stream_sparse_recv_holeHandler,
+ opaque) < 0)
+  _croak_error();
+
+  SvREFCNT_dec(opaque);
+
+
 void
 add_callback(stref, events, cb)
   SV* stref;
diff --git a/lib/Sys/Virt/Stream.pm b/lib/Sys/Virt/Stream.pm
index 5984fb6..c32b957 100644
--- a/lib/Sys/Virt/Stream.pm
+++ b/lib/Sys/Virt/Stream.pm
@@ -130,6 +130,20 @@ data byte count desired. The function should return
 the number of bytes filled, 0 on end of file, or
 -1 upon error
 
+=item $st->sparse_recv_all($handler, $holeHandler)
+
+Receive all data available from the sparse stream, invoking
+C<$handler> to process the data. The C<$handler> parameter must
+be a function which expects three arguments, the C<$st> stream
+object, a scalar containing the data received and a data byte
+count. The function should return the number of bytes processed,
+or -1 upon error. The second argument C<$holeHandler> is a
+function which expects two arguments: the C<$st> stream and a
+scalar, number describing the size of the hole in the stream (in
+bytes). The C<$holeHandler> is expected to return a non-negative
+number on success (usually 0) and a negative number (usually -1)
+otherwise.
+
 =item $st->add_callback($events, $coderef)
 
 Register a callback to be invoked whenever the stream has
diff --git a/t/030-api-coverage.t b/t/030-api-coverage.t
index 3049713..6a281ba 100644
--- a/t/030-api-coverage.t
+++ b/t/030-api-coverage.t
@@ -114,6 +114,7 @@ virEventUpdateTimeoutFunc
 
 virStreamEventCallback
 virStreamSinkFunc
+virStreamSinkHoleFunc
 virStreamSourceFunc
 
 virConnectCloseFunc
-- 
2.13.0

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

[libvirt] [libvirt-perl][PATCH 6/7] Register VOL_DOWNLOAD_SPARSE_STREAM & VOL_UPLOAD_SPARSE_STREAM constants

[Michal Privoznik]

Signed-off-by: Michal Privoznik 
---
 Changes|  2 ++
 Virt.xs|  4 
 lib/Sys/Virt/StorageVol.pm | 30 ++
 3 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/Changes b/Changes
index 989b795..6c671c0 100644
--- a/Changes
+++ b/Changes
@@ -9,6 +9,8 @@ Revision history for perl module Sys::Virt
  - Introduce Stream::recvHole() and Stream::sendHole()
  - Introduce Stream::sparse_recv_all()
  - Introduce Stream::sparse_send_all()
+ - Register VOL_DOWNLOAD_SPARSE_STREAM &
+   VOL_UPLOAD_SPARSE_STREAM constants
 
 3.3.0 2017-05-08
 
diff --git a/Virt.xs b/Virt.xs
index fc282b8..663571c 100644
--- a/Virt.xs
+++ b/Virt.xs
@@ -9209,6 +9209,10 @@ BOOT:
   REGISTER_CONSTANT(VIR_STORAGE_VOL_USE_ALLOCATION, USE_ALLOCATION);
   REGISTER_CONSTANT(VIR_STORAGE_VOL_GET_PHYSICAL, GET_PHYSICAL);
 
+  REGISTER_CONSTANT(VIR_STORAGE_VOL_DOWNLOAD_SPARSE_STREAM, 
VOL_DOWNLOAD_SPARSE_STREAM);
+
+  REGISTER_CONSTANT(VIR_STORAGE_VOL_UPLOAD_SPARSE_STREAM, 
VOL_UPLOAD_SPARSE_STREAM);
+
   stash = gv_stashpv( "Sys::Virt::Secret", TRUE );
   REGISTER_CONSTANT(VIR_SECRET_USAGE_TYPE_NONE, USAGE_TYPE_NONE);
   REGISTER_CONSTANT(VIR_SECRET_USAGE_TYPE_VOLUME, USAGE_TYPE_VOLUME);
diff --git a/lib/Sys/Virt/StorageVol.pm b/lib/Sys/Virt/StorageVol.pm
index 42f10e8..82c3df2 100644
--- a/lib/Sys/Virt/StorageVol.pm
+++ b/lib/Sys/Virt/StorageVol.pm
@@ -141,17 +141,39 @@ Return the physical size in allocation
 
 =back
 
-=item $vol->download($st, $offset, $length);
+=item $vol->download($st, $offset, $length, $flags=0);
 
 Download data from C<$vol> using the stream C<$st>. If C<$offset>
 and C<$length> are non-zero, then restrict data to the specified
-volume byte range.
+volume byte range. The C<$flags> accept the following values:
 
-=item $vol->upload($st, $offset, $length);
+=over 4
+
+=item Sys::Virt::StorageVol::VOL_DOWNLOAD_SPARSE_STREAM
+
+If this flag is is set in @flags effective transmission of holes
+is enabled. This assumes using the stream C<$st> with combination of
+C or C for honouring holes sent by server.
+
+=back
+
+=item $vol->upload($st, $offset, $length, $flags=0);
 
 Upload data to C<$vol> using the stream C<$st>. If C<$offset>
 and C<$length> are non-zero, then restrict data to the specified
-volume byte range.
+volume byte range. The C<$flags> accept the following values:
+
+=over 4
+
+=item Sys::Virt::StorageVol::VOL_UPLOAD_SPARSE_STREAM
+
+If this is set in C<$flags> effective transmission of holes is
+enabled. This assumes using the stream C<$st> with combination of
+C or C to preserve source file
+sparseness.
+
+=back
 
 =back
 
-- 
2.13.0

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

[libvirt] [libvirt-perl][PATCH 7/7] examples: Introduce vol-sparse.pl

[Michal Privoznik]

Signed-off-by: Michal Privoznik 
---
 Changes|   1 +
 examples/vol-sparse.pl | 142 +
 2 files changed, 143 insertions(+)
 create mode 100755 examples/vol-sparse.pl

diff --git a/Changes b/Changes
index 6c671c0..3742a8c 100644
--- a/Changes
+++ b/Changes
@@ -11,6 +11,7 @@ Revision history for perl module Sys::Virt
  - Introduce Stream::sparse_send_all()
  - Register VOL_DOWNLOAD_SPARSE_STREAM &
VOL_UPLOAD_SPARSE_STREAM constants
+ - Add vol-sparse.pl example
 
 3.3.0 2017-05-08
 
diff --git a/examples/vol-sparse.pl b/examples/vol-sparse.pl
new file mode 100755
index 000..b99b57f
--- /dev/null
+++ b/examples/vol-sparse.pl
@@ -0,0 +1,142 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use Sys::Virt;
+use Fcntl;
+
+my $FILE;
+
+sub downloadHandler {
+   my $st = shift;
+   my $data = shift;
+   my $nbytes = shift;
+   return syswrite FILE, $data, $nbytes;
+}
+
+sub downloadHoleHandler {
+my $st = shift;
+my $offset = shift;
+my $pos = sysseek FILE, $offset, Fcntl::SEEK_CUR or die "Unable to seek in 
$FILE: $!";
+truncate FILE, $pos;
+}
+
+
+sub download {
+my $vol = shift;
+my $st = shift;
+my $filename = shift;
+my $offset = 0;
+my $length = 0;
+
+open FILE, ">$filename" or die "unable to create $filename: $!";
+eval {
+$vol->download($st, $offset, $length, 
Sys::Virt::StorageVol::VOL_DOWNLOAD_SPARSE_STREAM);
+$st->sparse_recv_all(\, \);
+$st->finish();
+};
+if ($@) {
+unlink $filename if $@;
+close FILE;
+die $@;
+}
+
+close FILE or die "cannot save $filename: $!"
+}
+
+sub uploadHandler {
+my $st = $_[0];
+my $nbytes = $_[2];
+return sysread FILE, $_[1], $nbytes;
+}
+
+sub uploadHoleHandler {
+my $st = shift;
+my $inData;
+my $sectionLen;
+
+# HACK, Perl lacks SEEK_DATA and SEEK_HOLE.
+my $SEEK_DATA = 3;
+my $SEEK_HOLE = 4;
+
+my $cur = sysseek FILE, 0, Fcntl::SEEK_CUR;
+eval {
+my $data = sysseek FILE, $cur, $SEEK_DATA;
+# There are three options:
+# 1) $data == $cur;  $cur is in data
+# 2) $data > $cur; $cur is in a hole, next data at $data
+# 3) $data < 0; either $cur is in trailing hole, or $cur is beyond EOF.
+
+if (!defined($data)) {
+# case 3
+$inData = 0;
+my $end = sysseek FILE, 0, Fcntl::SEEK_END or die "Unable to get 
EOF position: $!";
+$sectionLen = $end - $cur;
+} elsif ($data > $cur) {
+#case 2
+$inData = 0;
+$sectionLen = $data - $cur;
+} else {
+#case 1
+my $hole = sysseek FILE, $data, $SEEK_HOLE;
+if (!defined($hole) or $hole eq $data) {
+die "Blah";
+}
+$inData = 1;
+$sectionLen = $hole - $data;
+}
+};
+
+# reposition file back
+sysseek FILE, $cur, Fcntl::SEEK_SET;
+
+return ($inData, $sectionLen);
+}
+
+sub uploadSkipHandler {
+my $st = shift;
+my $offset = shift;
+print("uploadSkipHandler: $offset\n");
+sysseek FILE, $offset, Fcntl::SEEK_CUR or die "Unable to seek in $FILE";
+}
+
+sub upload {
+my $vol = shift;
+my $st = shift;
+my $filename = shift;
+my $offset = 0;
+my $length = 0;
+
+open FILE, "<$filename" or die "unable to open $filename: $!";
+eval {
+$vol->upload($st, $offset, $length, 
Sys::Virt::StorageVol::VOL_UPLOAD_SPARSE_STREAM);
+$st->sparse_send_all(\, \, 
\);
+$st->finish();
+};
+if ($@) {
+close FILE;
+die $@;
+}
+
+close FILE or die "cannot close $filename: $!"
+}
+
+die "syntax: $0 URI --download/--upload VOLUME FILE" unless int(@ARGV) == 4;
+
+my $uri = shift @ARGV;
+my $action = shift @ARGV;
+my $volpath = shift @ARGV;
+my $filename = shift @ARGV;
+
+my $c = Sys::Virt->new(uri => $uri) or die "Unable to connect to $uri";
+my $vol = $c->get_storage_volume_by_key($volpath) or die "No such volume";
+my $st = $c->new_stream();
+
+if ($action eq "--download") {
+download($vol, $st, $filename);
+} elsif ($action eq "--upload") {
+upload($vol, $st, $filename);
+} else {
+die "unknown action $action";
+}
-- 
2.13.0

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

[libvirt] [libvirt-perl][PATCH 2/7] Introduce flags to Stream::recv()

[Michal Privoznik]

At the same time register RECV_STOP_AT_HOLE constant.

Signed-off-by: Michal Privoznik 
---
 Changes|  2 ++
 Virt.xs| 13 ++---
 lib/Sys/Virt/Stream.pm | 20 
 3 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/Changes b/Changes
index 2e4a99e..b4a493c 100644
--- a/Changes
+++ b/Changes
@@ -4,6 +4,8 @@ Revision history for perl module Sys::Virt
 
  - Add LIST_CAP_MDEV & LIST_CAP_MDEV_TYPES constants
  - Fix send_all() callback helper
+ - Introduce flags to Stream::recv() and
+   register RECV_STOP_AT_HOLE constant
 
 3.3.0 2017-05-08
 
diff --git a/Virt.xs b/Virt.xs
index a041c95..498e711 100644
--- a/Virt.xs
+++ b/Virt.xs
@@ -7874,16 +7874,21 @@ send(st, data, nbytes)
 
 
 int
-recv(st, data, nbytes)
+recv(st, data, nbytes, flags=0)
   virStreamPtr st;
   SV *data;
   size_t nbytes;
+  unsigned int flags;
  PREINIT:
   char *rawdata;
 CODE:
   Newx(rawdata, nbytes, char);
-  if ((RETVAL = virStreamRecv(st, rawdata, nbytes)) < 0 &&
-  RETVAL != -2) {
+  if (flags)
+  RETVAL = virStreamRecvFlags(st, rawdata, nbytes, flags);
+  else
+  RETVAL = virStreamRecv(st, rawdata, nbytes);
+
+  if (RETVAL != -2 && RETVAL != -3) {
   Safefree(rawdata);
   _croak_error();
   }
@@ -9010,6 +9015,8 @@ BOOT:
   REGISTER_CONSTANT(VIR_STREAM_EVENT_ERROR, EVENT_ERROR);
   REGISTER_CONSTANT(VIR_STREAM_EVENT_HANGUP, EVENT_HANGUP);
 
+  REGISTER_CONSTANT(VIR_STREAM_RECV_STOP_AT_HOLE, RECV_STOP_AT_HOLE);
+
 
 
   stash = gv_stashpv( "Sys::Virt::Error", TRUE );
diff --git a/lib/Sys/Virt/Stream.pm b/lib/Sys/Virt/Stream.pm
index 4022c84..069895e 100644
--- a/lib/Sys/Virt/Stream.pm
+++ b/lib/Sys/Virt/Stream.pm
@@ -69,11 +69,23 @@ be called on any stream which has been activated
 Complete I/O on the stream. Either this function or C must
 be called on any stream which has been activated
 
-=item $rv = $st->recv($data, $nbytes)
+=item $rv = $st->recv($data, $nbytes, $flags=0)
 
-Receive upto C<$nbytes> worth of data, copying into C<$data>.
-Returns the number of bytes read, or -2 if I/O would block,
-or -1 on error.
+Receive up to C<$nbytes> worth of data, copying into C<$data>.
+Returns the number of bytes read, or -3 if hole is reached and
+C<$flags> contains RECV_STOP_AT_HOLE, or -2 if I/O would block,
+or -1 on error. The C<$flags> parameter accepts the following
+flags:
+
+=over 4
+
+=item Sys::Virt::Stream::RECV_STOP_AT_HOLE
+
+If this flag is set, the C function will stop reading from
+stream if it has reached a hole. In that case, -3 is returned and
+C should be called to get the hole size.
+
+=back
 
 =item $rv = $st->send($data, $nbytes)
 
-- 
2.13.0

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

[libvirt] [libvirt-perl][PATCH 3/7] Introduce Stream::recvHole() and Stream::sendHole()

[Michal Privoznik]

Signed-off-by: Michal Privoznik 
---
 Changes|  1 +
 Virt.xs| 28 
 lib/Sys/Virt/Stream.pm | 17 +
 3 files changed, 46 insertions(+)

diff --git a/Changes b/Changes
index b4a493c..c92c271 100644
--- a/Changes
+++ b/Changes
@@ -6,6 +6,7 @@ Revision history for perl module Sys::Virt
  - Fix send_all() callback helper
  - Introduce flags to Stream::recv() and
register RECV_STOP_AT_HOLE constant
+ - Introduce Stream::recvHole() and Stream::sendHole()
 
 3.3.0 2017-05-08
 
diff --git a/Virt.xs b/Virt.xs
index 498e711..d112708 100644
--- a/Virt.xs
+++ b/Virt.xs
@@ -7900,6 +7900,34 @@ recv(st, data, nbytes, flags=0)
   RETVAL
 
 
+SV *
+recvHole(st, flags=0)
+  virStreamPtr st;
+  unsigned int flags;
+  PREINIT:
+  long long length;
+CODE:
+  if (virStreamRecvHole(st, , flags) < 0)
+  _croak_error();
+
+  RETVAL = virt_newSVll(length);
+  OUTPUT:
+  RETVAL
+
+
+void
+sendHole(st, lengthSV, flags=0)
+  virStreamPtr st;
+  SV *lengthSV;
+  unsigned int flags;
+ PREINIT:
+  long long length;
+  PPCODE:
+  length = virt_SvIVll(lengthSV);
+  if (virStreamSendHole(st, length, flags) < 0)
+  _croak_error();
+
+
 void
 send_all(stref, handler)
   SV *stref;
diff --git a/lib/Sys/Virt/Stream.pm b/lib/Sys/Virt/Stream.pm
index 069895e..5984fb6 100644
--- a/lib/Sys/Virt/Stream.pm
+++ b/lib/Sys/Virt/Stream.pm
@@ -93,6 +93,23 @@ Send upto C<$nbytes> worth of data, copying from C<$data>.
 Returns the number of bytes sent, or -2 if I/O would block,
 or -1 on error.
 
+=item $rv = $st->recvHole($flags=0)
+
+Determine the amount of the empty space (in bytes) to be created
+in a stream's target file when uploading or downloading sparsely
+populated files. This is the counterpart to C. The
+optional C<$flags> parameter is currently unused and defaults to
+zero if omitted.
+
+=item $st->sendHole($length, $flags=0)
+
+Rather than transmitting empty file space, this method directs
+the stream target to create C<$length> bytes of empty space.
+This method would be used when uploading or downloading sparsely
+populated files to avoid the needless copy of empty file space.
+The optional C<$flags> parameter is currently unused and defaults
+to zero if omitted.
+
 =item $st->recv_all($handler)
 
 Receive all data available from the stream, invoking
-- 
2.13.0

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

[libvirt] [libvirt-perl][PATCH 5/7] Introduce Stream::sparse_send_all()

[Michal Privoznik]

Signed-off-by: Michal Privoznik 
---
 Changes|   1 +
 Virt.xs| 124 +
 lib/Sys/Virt/Stream.pm |  19 
 t/030-api-coverage.t   |   2 +
 4 files changed, 146 insertions(+)

diff --git a/Changes b/Changes
index 4c1e071..989b795 100644
--- a/Changes
+++ b/Changes
@@ -8,6 +8,7 @@ Revision history for perl module Sys::Virt
register RECV_STOP_AT_HOLE constant
  - Introduce Stream::recvHole() and Stream::sendHole()
  - Introduce Stream::sparse_recv_all()
+ - Introduce Stream::sparse_send_all()
 
 3.3.0 2017-05-08
 
diff --git a/Virt.xs b/Virt.xs
index 002bd6a..fc282b8 100644
--- a/Virt.xs
+++ b/Virt.xs
@@ -1960,6 +1960,100 @@ _stream_send_all_source(virStreamPtr st,
 }
 
 
+static int
+_stream_sparse_send_all_holeHandler(virStreamPtr st,
+int *inData,
+long long *length,
+void *opaque)
+{
+AV *av = opaque;
+SV **self;
+SV **holeHandler;
+SV *inDataSV;
+SV *lengthSV;
+int count;
+int ret;
+dSP;
+
+self = av_fetch(av, 0, 0);
+holeHandler = av_fetch(av, 2, 0);
+
+SvREFCNT_inc(*self);
+
+ENTER;
+SAVETMPS;
+
+PUSHMARK(SP);
+XPUSHs(*self);
+PUTBACK;
+
+count = call_sv((SV*)*holeHandler, G_ARRAY);
+
+SPAGAIN;
+
+if (count == 2) {
+/* @holeHandler returns (inData, length), but on a stack.
+ * Therefore the order is reversed. */
+lengthSV = POPs;
+inDataSV = POPs;
+*inData = virt_SvIVll(inDataSV);
+*length = virt_SvIVll(lengthSV);
+ret = 0;
+} else {
+ret = -1;
+}
+
+PUTBACK;
+FREETMPS;
+LEAVE;
+
+return ret;
+}
+
+
+static int
+_stream_sparse_send_all_skipHandler(virStreamPtr st,
+long long length,
+void *opaque)
+{
+AV *av = opaque;
+SV **self;
+SV **skipHandler;
+int rv;
+int ret;
+dSP;
+
+self = av_fetch(av, 0, 0);
+skipHandler = av_fetch(av, 3, 0);
+
+SvREFCNT_inc(*self);
+
+ENTER;
+SAVETMPS;
+
+PUSHMARK(SP);
+XPUSHs(*self);
+XPUSHs(sv_2mortal(virt_newSVll(length)));
+PUTBACK;
+
+rv = call_sv((SV*)*skipHandler, G_SCALAR);
+
+SPAGAIN;
+
+if (rv == 1) {
+ret = POPi;
+} else {
+ret = -1;
+}
+
+PUTBACK;
+FREETMPS;
+LEAVE;
+
+return ret;
+}
+
+
 static int
 _stream_recv_all_sink(virStreamPtr st,
   const char *data,
@@ -8041,6 +8135,36 @@ sparse_recv_all(stref, handler, holeHandler)
 
   SvREFCNT_dec(opaque);
 
+void
+sparse_send_all(stref, handler, holeHandler, skipHandler)
+  SV *stref;
+  SV *handler;
+  SV *holeHandler;
+  SV *skipHandler;
+ PREINIT:
+  AV *opaque;
+  virStreamPtr st;
+CODE:
+  st = (virStreamPtr)SvIV((SV*)SvRV(stref));
+
+  opaque = newAV();
+  SvREFCNT_inc(stref);
+  SvREFCNT_inc(handler);
+  SvREFCNT_inc(holeHandler);
+  SvREFCNT_inc(skipHandler);
+  av_push(opaque, stref);
+  av_push(opaque, handler);
+  av_push(opaque, holeHandler);
+  av_push(opaque, skipHandler);
+
+  if (virStreamSparseSendAll(st,
+ _stream_send_all_source,
+ _stream_sparse_send_all_holeHandler,
+ _stream_sparse_send_all_skipHandler,
+ opaque) < 0)
+  _croak_error();
+
+  SvREFCNT_dec(opaque);
 
 void
 add_callback(stref, events, cb)
diff --git a/lib/Sys/Virt/Stream.pm b/lib/Sys/Virt/Stream.pm
index c32b957..e3b25a5 100644
--- a/lib/Sys/Virt/Stream.pm
+++ b/lib/Sys/Virt/Stream.pm
@@ -144,6 +144,25 @@ bytes). The C<$holeHandler> is expected to return a 
non-negative
 number on success (usually 0) and a negative number (usually -1)
 otherwise.
 
+=item $st->sparse_send_all($handler, $holeHandler, $skipHandler)
+
+Send all data produced by C<$handler> to the stream.  The
+C<$handler> parameter must be a function which expects three
+arguments, the C<$st> stream object, a scalar which must be
+filled with data and a maximum data byte count desired.  The
+function should return the number of bytes filled, 0 on end of
+file, or -1 upon error. The second argument C<$holeHandler> is a
+function expecting just one argument C<$st> and returning an
+array of two elements (C<$inData>, C<$sectionLen>) where
+C<$inData> has zero or non-zero value if underlying file is in a
+hole or data section respectively. The C<$sectionLen> then is the
+number of remaining bytes in the current section in the
+underlying file. Finally, the third C<$skipHandler> is a function
+expecting two arguments C<$st> and C<$length> which moves cursor
+in the underlying file for C<$length> bytes. The C<$skipHandler>
+is expected to return a non-negative number on success (usually

[libvirt] [libvirt-perl][PATCH 0/7] Implement sparse streams

[Michal Privoznik]

*** BLURB HERE ***

Michal Privoznik (7):
  Fix send_all() callback helper
  Introduce flags to Stream::recv()
  Introduce Stream::recvHole() and Stream::sendHole()
  Introduce Stream::sparse_recv_all()
  Introduce Stream::sparse_send_all()
  Register VOL_DOWNLOAD_SPARSE_STREAM & VOL_UPLOAD_SPARSE_STREAM
constants
  examples: Introduce vol-sparse.pl

 Changes|   9 ++
 Virt.xs| 241 -
 examples/vol-sparse.pl | 142 ++
 lib/Sys/Virt/StorageVol.pm |  30 +-
 lib/Sys/Virt/Stream.pm |  70 -
 t/030-api-coverage.t   |   3 +
 6 files changed, 483 insertions(+), 12 deletions(-)
 create mode 100755 examples/vol-sparse.pl

-- 
2.13.0

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

[libvirt] [libvirt-perl][PATCH 1/7] Fix send_all() callback helper

[Michal Privoznik]

Sys::Virt::virStream->send_all() uses virStreamSendAll() under
the hood. This function takes one callback to fill the send
buffer with stream data. We have a C glue callback that
eventually calls the perl one. However, there's a problem with
the glue callback mangling the data as it mistakenly uses
strcpy() instead of memcpy().

Signed-off-by: Michal Privoznik 
---
 Changes | 1 +
 Virt.xs | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Changes b/Changes
index 2e5bfe4..2e4a99e 100644
--- a/Changes
+++ b/Changes
@@ -3,6 +3,7 @@ Revision history for perl module Sys::Virt
 3.4.0 2017-06-00
 
  - Add LIST_CAP_MDEV & LIST_CAP_MDEV_TYPES constants
+ - Fix send_all() callback helper
 
 3.3.0 2017-05-08
 
diff --git a/Virt.xs b/Virt.xs
index 9ccdc1f..a041c95 100644
--- a/Virt.xs
+++ b/Virt.xs
@@ -1948,7 +1948,7 @@ _stream_send_all_source(virStreamPtr st,
 const char *newdata = SvPV_nolen(datasv);
 if (ret > nbytes)
 ret = nbytes;
-strncpy(data, newdata, nbytes);
+memcpy(data, newdata, nbytes);
 }
 
 FREETMPS;
-- 
2.13.0

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

[libvirt] not able install libvirt

[Eric Blake]

[moderator note: re-posting to list with screenshot deleted, in order to
meet list policy size constraints]

hello,
i am am trying to install libvmi tool on my xen hypervisor but some where
is asking to install libvirt packages.
i am following your code and your instruction but its showing some error
here is screenshot.
how i can resolve it ??




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

Re: [libvirt] [PATCH v2] lib: Add note that bulk stats API queries may overrun RPC buffers

[Martin Kletzander]

On Tue, May 23, 2017 at 04:23:30PM +0200, Peter Krempa wrote:

Hint that the users should limit the number of VMs queried in the bulk
stats API.
---
v2:
- added a suggestion of the number of queried VMs (valid after bump to 32M 
message)

src/libvirt-domain.c | 8 
1 file changed, 8 insertions(+)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 310b91b37..aef061943 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11315,6 +11315,10 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
 * VIR_CONNECT_GET_ALL_DOMAINS_STATS_SHUTOFF and/or
 * VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER for all other states.
 *
+ * Note that this API is prone to exceeding maximum RPC message size on hosts
+ * with lots of VMs so it's suggested to use virDomainListGetStats with a
+ * reasonable list of VMs as the argument.
+ *
 * Returns the count of returned statistics structures on success, -1 on error.
 * The requested data are returned in the @retStats parameter. The returned
 * array should be freed by the caller. See virDomainStatsRecordListFree.
@@ -11386,6 +11390,10 @@ virConnectGetAllDomainStats(virConnectPtr conn,
 * Note that any of the domain list filtering flags in @flags may be rejected
 * by this function.
 *
+ * Note that this API is prone to exceeding maximum RPC if querying too many 
VMs
+ * with lots of statistics. It's suggested to query in batches of 10VMs, which
+ * should be good enough for VMs with 3000 disks + networks.
+ *


Coming to think about it... Why don't we just batch this ourselves under
the hood and just return the merged result?


 * Returns the count of returned statistics structures on success, -1 on error.
 * The requested data are returned in the @retStats parameter. The returned
 * array should be freed by the caller. See virDomainStatsRecordListFree.
--
2.12.2

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


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

[libvirt] [libvirt-python][PATCH v2 0/4] Implement sparse streams

[Michal Privoznik]

diff to v1:
- Martin's review worked in.

Patches 1-3 were ACKed already. Well, conditionally. Patch 4/4 wasn't. I'm
sending them again to make sure I've worked in the review as expected.

Michal Privoznik (4):
  Implement virStreamSendHole/virStreamRecvHole
  virStream: Introduce virStreamRecvFlags
  virStream: Introduce virStreamSparse{Recv,Send}All
  examples: Introduce sparsestream.py

 examples/sparsestream.py  | 117 +
 generator.py  |   5 ++
 libvirt-override-virStream.py | 146 ++
 libvirt-override.c| 102 +
 sanitytest.py |   6 +-
 5 files changed, 374 insertions(+), 2 deletions(-)
 create mode 100755 examples/sparsestream.py

-- 
2.13.0

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

Re: [libvirt] [PATCH] qemu: process: Clear priv->namespaces on VM shutdown

[Peter Krempa]

On Mon, May 22, 2017 at 15:04:58 +0200, Martin Kletzander wrote:
> On Mon, May 22, 2017 at 01:40:01PM +0200, Peter Krempa wrote:
> > Otherwise the private data entry would be kept across instances of the
> > same VM even if it's not configured to do so.
> > 
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1453142
> > ---
> > src/qemu/qemu_process.c | 4 
> > 1 file changed, 4 insertions(+)
> > 
> > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> > index c19bd2925..ac17da668 100644
> > --- a/src/qemu/qemu_process.c
> > +++ b/src/qemu/qemu_process.c
> > @@ -6449,6 +6449,10 @@ void qemuProcessStop(virQEMUDriverPtr driver,
> > /* clean up migration data */
> > VIR_FREE(priv->migTLSAlias);
> > 
> > +/* clear previously used namespaces */
> > +virBitmapFree(priv->namespaces);
> > +priv->namespaces = NULL;
> > +
> 
> Does this mean we can kill the call to qemuDomainDestroyNamespace() from
> qemuProcessHandleMonitorEOF() (and possibly the whole function)?

The comment there hints that it may be necessary in some cleanup cases,
so I did not remove it.

> 
> ACK either way, this should be cleaned up every time.

Thanks, there are no cleanup steps after that so clearing it is okay
either way.


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

[libvirt] [libvirt-python][PATCH v2 1/4] Implement virStreamSendHole/virStreamRecvHole

[Michal Privoznik]

The return value for virStreamRecvHole is slightly different to
its C counterpart. In python, either it returns the hole size or
None if C API fails.

Signed-off-by: Michal Privoznik 
---
 generator.py  |  2 ++
 libvirt-override-virStream.py | 21 +++
 libvirt-override.c| 63 +++
 3 files changed, 86 insertions(+)

diff --git a/generator.py b/generator.py
index 5dfa73e..ca1df35 100755
--- a/generator.py
+++ b/generator.py
@@ -543,6 +543,8 @@ skip_function = (
 'virStreamSendAll', # Pure python libvirt-override-virStream.py
 'virStreamRecv', # overridden in libvirt-override-virStream.py
 'virStreamSend', # overridden in libvirt-override-virStream.py
+'virStreamRecvHole', # overridden in libvirt-override-virStream.py
+'virStreamSendHole', # overridden in libvirt-override-virStream.py
 
 'virConnectUnregisterCloseCallback', # overridden in virConnect.py
 'virConnectRegisterCloseCallback', # overridden in virConnect.py
diff --git a/libvirt-override-virStream.py b/libvirt-override-virStream.py
index 2e77cc7..62c1328 100644
--- a/libvirt-override-virStream.py
+++ b/libvirt-override-virStream.py
@@ -125,3 +125,24 @@
 ret = libvirtmod.virStreamSend(self._o, data)
 if ret == -1: raise libvirtError ('virStreamSend() failed')
 return ret
+
+def recvHole(self, flags = 0):
+"""This method is used to determine the length in bytes
+of the empty space to be created in a stream's target
+file when uploading or downloading sparsely populated
+files. This is the counterpart to sendHole.
+"""
+ret = libvirtmod.virStreamRecvHole(self._o, flags)
+if ret is None: raise libvirtError ('virStreamRecvHole() failed')
+return ret
+
+def sendHole(self, length, flags = 0):
+"""Rather than transmitting empty file space, this method
+directs the stream target to create length bytes of empty
+space.  This method would be used when uploading or
+downloading sparsely populated files to avoid the
+needless copy of empty file space.
+"""
+ret = libvirtmod.virStreamSendHole(self._o, length, flags)
+if ret == -1: raise libvirtError('virStreamSendHole() failed')
+return ret
diff --git a/libvirt-override.c b/libvirt-override.c
index a762941..a6ef49e 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -9463,6 +9463,65 @@ libvirt_virConnectSecretEventDeregisterAny(PyObject 
*self ATTRIBUTE_UNUSED,
 }
 #endif /* LIBVIR_CHECK_VERSION(3, 0, 0)*/
 
+
+#if LIBVIR_CHECK_VERSION(3, 4, 0)
+static PyObject *
+libvirt_virStreamRecvHole(PyObject *self ATTRIBUTE_UNUSED,
+  PyObject *args)
+{
+PyObject *pyobj_stream;
+virStreamPtr stream;
+long long length = -1;
+unsigned int flags;
+int ret;
+
+if (!PyArg_ParseTuple(args, (char *) "OI:virStreamRecvHole",
+  _stream, ))
+return NULL;
+
+stream = PyvirStream_Get(pyobj_stream);
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+ret = virStreamRecvHole(stream, , flags);
+LIBVIRT_END_ALLOW_THREADS;
+
+DEBUG("StreamRecvHole ret=%d length=%lld\n", ret, length);
+
+if (ret < 0)
+return VIR_PY_NONE;
+
+return libvirt_longlongWrap(length);
+}
+
+
+static PyObject *
+libvirt_virStreamSendHole(PyObject *self ATTRIBUTE_UNUSED,
+  PyObject *args)
+{
+PyObject *pyobj_stream;
+virStreamPtr stream;
+long long length;
+unsigned int flags;
+int ret;
+
+if (!PyArg_ParseTuple(args, (char *) "OLI:virStreamSendHole",
+  _stream, , ))
+return NULL;
+
+stream = PyvirStream_Get(pyobj_stream);
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+ret = virStreamSendHole(stream, length, flags);
+LIBVIRT_END_ALLOW_THREADS;
+
+DEBUG("StreamSendHole ret=%d\n", ret);
+
+return libvirt_intWrap(ret);
+}
+
+#endif /* LIBVIR_CHECK_VERSION(3, 4, 0) */
+
+
 /
  * *
  * The registration stuff  *
@@ -9687,6 +9746,10 @@ static PyMethodDef libvirtMethods[] = {
 {(char *) "virConnectSecretEventRegisterAny", 
libvirt_virConnectSecretEventRegisterAny, METH_VARARGS, NULL},
 {(char *) "virConnectSecretEventDeregisterAny", 
libvirt_virConnectSecretEventDeregisterAny, METH_VARARGS, NULL},
 #endif /* LIBVIR_CHECK_VERSION(3, 0, 0) */
+#if LIBVIR_CHECK_VERSION(3, 4, 0)
+{(char *) "virStreamRecvHole", libvirt_virStreamRecvHole, METH_VARARGS, 
NULL},
+{(char *) "virStreamSendHole", libvirt_virStreamSendHole, METH_VARARGS, 
NULL},
+#endif /* LIBVIR_CHECK_VERSION(3, 4, 0) */
 {NULL, NULL, 0, NULL}
 };
 
-- 
2.13.0

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

[libvirt] [libvirt-python][PATCH v2 3/4] virStream: Introduce virStreamSparse{Recv, Send}All

[Michal Privoznik]

Yet again, our parser is not capable of generating proper
wrapper. To be fair, this one wold be really tough anyway.

Signed-off-by: Michal Privoznik 
---
 generator.py  |   2 +
 libvirt-override-virStream.py | 107 ++
 sanitytest.py |   6 ++-
 3 files changed, 113 insertions(+), 2 deletions(-)

diff --git a/generator.py b/generator.py
index 0e07fc8..93d1dc3 100755
--- a/generator.py
+++ b/generator.py
@@ -546,6 +546,8 @@ skip_function = (
 'virStreamRecvHole', # overridden in libvirt-override-virStream.py
 'virStreamSendHole', # overridden in libvirt-override-virStream.py
 'virStreamRecvFlags', # overridden in libvirt-override-virStream.py
+'virStreamSparseRecvAll', # overridden in libvirt-override-virStream.py
+'virStreamSparseSendAll', # overridden in libvirt-override-virStream.py
 
 'virConnectUnregisterCloseCallback', # overridden in virConnect.py
 'virConnectRegisterCloseCallback', # overridden in virConnect.py
diff --git a/libvirt-override-virStream.py b/libvirt-override-virStream.py
index 66d2bf6..0ab7815 100644
--- a/libvirt-override-virStream.py
+++ b/libvirt-override-virStream.py
@@ -164,3 +164,110 @@
 ret = libvirtmod.virStreamRecvFlags(self._o, nbytes, flags)
 if ret is None: raise libvirtError ('virStreamRecvFlags() failed')
 return ret
+
+def sparseRecvAll(self, handler, holeHandler, opaque):
+"""Receive the entire data stream, sending the data to
+the requested data sink handler and calling the skip
+holeHandler to generate holes for sparse stream targets.
+This is simply a convenient alternative to recvFlags, for
+apps that do blocking-I/O and want to preserve sparseness.
+
+Hypothetical callbacks can look like this:
+
+def handler(stream, # virStream instance
+buf,# string containing received data
+opaque): # extra data passed to sparseRecvAll as opaque
+fd = opaque
+return os.write(fd, buf)
+
+def holeHandler(stream, # virStream instance
+length, # number of bytes to skip
+opaque): # extra data passed to sparseRecvAll as 
opaque
+fd = opaque
+cur = os.lseek(fd, length, os.SEEK_CUR)
+return os.ftruncate(fd, cur) # take this extra step to
+ # actually allocate the hole
+"""
+while True:
+want = 64 * 1024
+got = self.recvFlags(want, VIR_STREAM_RECV_STOP_AT_HOLE)
+if got == -2:
+raise libvirtError("cannot use sparseRecvAll with "
+   "nonblocking stream")
+if got == -3:
+length = self.recvHole()
+if length is None:
+self.abort()
+raise RuntimeError("recvHole handler failed")
+ret = holeHandler(self, length, opaque)
+if type(ret) is int and ret < 0:
+self.abort()
+raise RuntimeError("holeHandler handler returned %d" % ret)
+continue
+
+if len(got) == 0:
+break
+
+try:
+ret = handler(self, got, opaque)
+if type(ret) is int and ret < 0:
+raise RuntimeError("sparseRecvAll handler returned %d" % 
ret)
+except Exception as e:
+self.abort()
+raise e
+
+def sparseSendAll(self, handler, holeHandler, skipHandler, opaque):
+"""Send the entire data stream, reading the data from the
+requested data source. This is simply a convenient
+alternative to virStreamSend, for apps that do
+blocking-I/O and want to preserve sparseness.
+
+Hypothetical callbacks can look like this:
+
+def handler(stream, # virStream instance
+nbytes, # int amt of data to read
+opaque): # extra data passed to sparseSendAll as opaque
+fd = opaque
+return os.read(fd, nbytes)
+
+def holeHandler(stream, # virStream instance
+opaque): # extra data passed to sparseSendAll as 
opaque
+fd = opaque
+cur = os.lseek(fd, 0, os.SEEK_CUR)
+# ... find out current section and its boundaries
+# and set inData = True/False and sectionLen correspondingly
+os.lseek(fd, cur, os.SEEK_SET)
+return [inData, sectionLen]
+
+def skipHandler(stream, # virStream instance
+length, # number of bytes to skip
+opaque): # extra data passed to sparseSendAll as 
opaque
+fd 

[libvirt] [libvirt-python][PATCH v2 4/4] examples: Introduce sparsestream.py

[Michal Privoznik]

Sparse streams are not that straight forward to use for the very
first time. Especially the sparseRecvAll() and sparseSendAll()
methods which expects callbacks. What we can do to make it easier
for developers is to have an example where they can take an
inspiration from.

Signed-off-by: Michal Privoznik 
---
 examples/sparsestream.py | 117 +++
 1 file changed, 117 insertions(+)
 create mode 100755 examples/sparsestream.py

diff --git a/examples/sparsestream.py b/examples/sparsestream.py
new file mode 100755
index 000..e960c40
--- /dev/null
+++ b/examples/sparsestream.py
@@ -0,0 +1,117 @@
+#!/usr/bin/env python
+# Example of sparse streams usage
+#
+# Authors:
+#   Michal Privoznik 
+
+import libvirt, sys, os
+
+def bytesWriteHandler(stream, buf, opaque):
+fd = opaque
+return os.write(fd, buf)
+
+def bytesReadHandler(stream, nbytes, opaque):
+fd = opaque
+return os.read(fd, nbytes)
+
+def recvSkipHandler(stream, length, opaque):
+fd = opaque
+cur = os.lseek(fd, length, os.SEEK_CUR)
+return os.ftruncate(fd, cur)
+
+def sendSkipHandler(stream, length, opaque):
+fd = opaque
+return os.lseek(fd, length, os.SEEK_CUR)
+
+def holeHandler(stream, opaque):
+fd = opaque
+cur = os.lseek(fd, 0, os.SEEK_CUR)
+
+try:
+data = os.lseek(fd, cur, os.SEEK_DATA)
+except OSError as e:
+if e.errno != 6:
+raise e
+else:
+data = -1;
+# There are three options:
+# 1) data == cur;  @cur is in data
+# 2) data > cur; @cur is in a hole, next data at @data
+# 3) data < 0; either @cur is in trailing hole, or @cur is beyond EOF.
+if data < 0:
+# case 3
+inData = False
+eof = os.lseek(fd, 0, os.SEEK_END)
+if (eof < cur):
+raise RuntimeError("Current position in file after EOF: %d" % cur)
+sectionLen = eof - cur
+else:
+if (data > cur):
+# case 2
+inData = False
+sectionLen = data - cur
+else:
+# case 1
+inData = True
+
+# We don't know where does the next hole start. Let's find out.
+# Here we get the same options as above
+hole = os.lseek(fd, data, os.SEEK_HOLE)
+if hole < 0:
+# case 3. But wait a second. There is always a trailing hole.
+# Do the best what we can here
+raise RuntimeError("No trailing hole")
+
+if (hole == data):
+# case 1. Again, this is suspicious. The reason we are here is
+# because we are in data. But at the same time we are in a
+# hole. WAT?
+raise RuntimeError("Impossible happened")
+else:
+# case 2
+sectionLen = hole - data
+os.lseek(fd, cur, os.SEEK_SET)
+return [inData, sectionLen]
+
+def download(vol, st, filename):
+offset = 0
+length = 0
+
+fd = os.open(filename, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, mode=0o0660)
+vol.download(st, offset, length, 
libvirt.VIR_STORAGE_VOL_DOWNLOAD_SPARSE_STREAM)
+st.sparseRecvAll(bytesWriteHandler, recvSkipHandler, fd)
+
+os.close(fd)
+
+def upload(vol, st, filename):
+offset = 0
+length = 0
+
+fd = os.open(filename, os.O_RDONLY)
+vol.upload(st, offset, length, 
libvirt.VIR_STORAGE_VOL_UPLOAD_SPARSE_STREAM)
+st.sparseSendAll(bytesReadHandler, holeHandler, sendSkipHandler, fd)
+
+os.close(fd)
+
+# main
+if len(sys.argv) != 5:
+print("Usage: ", sys.argv[0], " URI --upload/--download VOLUME FILE")
+print("Either uploads local FILE to libvirt VOLUME, or downloads libvirt ")
+print("VOLUME into local FILE while preserving FILE/VOLUME sparseness")
+sys.exit(1)
+
+conn = libvirt.open(sys.argv[1])
+vol = conn.storageVolLookupByKey(sys.argv[3])
+
+st = conn.newStream()
+
+if sys.argv[2] == "--download":
+download(vol, st, sys.argv[4])
+elif sys.argv[2] == "--upload":
+upload(vol, st, sys.argv[4])
+else:
+print("Unknown operation: %s " % sys.argv[1])
+sys.exit(1)
+
+st.finish()
+conn.close()
-- 
2.13.0

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

[libvirt] [libvirt-python][PATCH v2 2/4] virStream: Introduce virStreamRecvFlags

[Michal Privoznik]

Yet again, we need a custom wrapper over virStreamRecvFlags
because our generator is not capable of generating it.

Signed-off-by: Michal Privoznik 
---
 generator.py  |  1 +
 libvirt-override-virStream.py | 18 ++
 libvirt-override.c| 39 +++
 3 files changed, 58 insertions(+)

diff --git a/generator.py b/generator.py
index ca1df35..0e07fc8 100755
--- a/generator.py
+++ b/generator.py
@@ -545,6 +545,7 @@ skip_function = (
 'virStreamSend', # overridden in libvirt-override-virStream.py
 'virStreamRecvHole', # overridden in libvirt-override-virStream.py
 'virStreamSendHole', # overridden in libvirt-override-virStream.py
+'virStreamRecvFlags', # overridden in libvirt-override-virStream.py
 
 'virConnectUnregisterCloseCallback', # overridden in virConnect.py
 'virConnectRegisterCloseCallback', # overridden in virConnect.py
diff --git a/libvirt-override-virStream.py b/libvirt-override-virStream.py
index 62c1328..66d2bf6 100644
--- a/libvirt-override-virStream.py
+++ b/libvirt-override-virStream.py
@@ -146,3 +146,21 @@
 ret = libvirtmod.virStreamSendHole(self._o, length, flags)
 if ret == -1: raise libvirtError('virStreamSendHole() failed')
 return ret
+
+def recvFlags(self, nbytes, flags = 0):
+"""Reads a series of bytes from the stream. This method may
+block the calling application for an arbitrary amount
+of time. This is just like recv except it has flags
+argument.
+
+Errors are not guaranteed to be reported synchronously
+with the call, but may instead be delayed until a
+subsequent call.
+
+On success, the received data is returned. On failure, an
+exception is raised. If the stream is a NONBLOCK stream and
+the request would block, integer -2 is returned.
+"""
+ret = libvirtmod.virStreamRecvFlags(self._o, nbytes, flags)
+if ret is None: raise libvirtError ('virStreamRecvFlags() failed')
+return ret
diff --git a/libvirt-override.c b/libvirt-override.c
index a6ef49e..0abfc37 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -9519,6 +9519,44 @@ libvirt_virStreamSendHole(PyObject *self 
ATTRIBUTE_UNUSED,
 return libvirt_intWrap(ret);
 }
 
+
+static PyObject *
+libvirt_virStreamRecvFlags(PyObject *self ATTRIBUTE_UNUSED,
+   PyObject *args)
+{
+PyObject *pyobj_stream;
+PyObject *rv;
+virStreamPtr stream;
+char *buf = NULL;
+size_t nbytes;
+unsigned int flags;
+int ret;
+
+if (!PyArg_ParseTuple(args, (char *) "OkI:virStreamRecvFlags",
+  _stream, , ))
+return NULL;
+
+stream = PyvirStream_Get(pyobj_stream);
+
+if (VIR_ALLOC_N(buf, nbytes + 1) < 0)
+return PyErr_NoMemory();
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+ret = virStreamRecvFlags(stream, buf, nbytes, flags);
+LIBVIRT_END_ALLOW_THREADS;
+
+buf[ret > -1 ? ret : 0] = '\0';
+DEBUG("StreamRecvFlags ret=%d strlen=%d\n", ret, (int) strlen(buf));
+
+if (ret == -2 || ret == -3)
+return libvirt_intWrap(ret);
+if (ret < 0)
+return VIR_PY_NONE;
+rv = libvirt_charPtrSizeWrap((char *) buf, (Py_ssize_t) ret);
+VIR_FREE(buf);
+return rv;
+}
+
 #endif /* LIBVIR_CHECK_VERSION(3, 4, 0) */
 
 
@@ -9749,6 +9787,7 @@ static PyMethodDef libvirtMethods[] = {
 #if LIBVIR_CHECK_VERSION(3, 4, 0)
 {(char *) "virStreamRecvHole", libvirt_virStreamRecvHole, METH_VARARGS, 
NULL},
 {(char *) "virStreamSendHole", libvirt_virStreamSendHole, METH_VARARGS, 
NULL},
+{(char *) "virStreamRecvFlags", libvirt_virStreamRecvFlags, METH_VARARGS, 
NULL},
 #endif /* LIBVIR_CHECK_VERSION(3, 4, 0) */
 {NULL, NULL, 0, NULL}
 };
-- 
2.13.0

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

Re: [libvirt] [PATCH 2/2] qemu: don't relabel chardev source file if virtlogd is used

[Martin Kletzander]

On Mon, May 15, 2017 at 04:28:35PM +0200, Pavel Hrdina wrote:

If libvirt uses virtlogd instead of passing the file path directly
to QEMU we shouldn't relabel the chardev source file, otherwise
virtlogd will get a permission denied while reloading.

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



You missed a digit and I'm too lazy to check 10 bugs for a reproducer ;)


Signed-off-by: Pavel Hrdina 
---
src/conf/domain_conf.c  | 20 
src/conf/domain_conf.h  |  1 +
src/qemu/qemu_command.c | 12 
src/security/security_dac.c |  6 ++
src/security/security_selinux.c |  6 ++
5 files changed, 41 insertions(+), 4 deletions(-)



I see you are changing the parser code, but you are not changing the
Relax-NG schema, neither any test.


diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index aa441fae3c..92f011d3a4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2064,6 +2064,7 @@ virDomainChrSourceDefCopy(virDomainChrSourceDefPtr dest,
}

dest->type = src->type;
+dest->skipRelabel = src->skipRelabel;

return 0;
}
@@ -10608,6 +10609,7 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr 
def,
char *append = NULL;
char *haveTLS = NULL;
char *tlsFromConfig = NULL;
+char *skipRelabel = NULL;
int remaining = 0;

while (cur != NULL) {
@@ -10628,6 +10630,8 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr 
def,
case VIR_DOMAIN_CHR_TYPE_UNIX:
if (!append && def->type == VIR_DOMAIN_CHR_TYPE_FILE)
append = virXMLPropString(cur, "append");
+if (!skipRelabel && def->type == VIR_DOMAIN_CHR_TYPE_FILE)
+skipRelabel = virXMLPropString(cur, "skipRelabel");


I'm guessing you want to keep this away from users, right?  You should
not be parsing it from the XML then.  Or you should add a thing there
that the XML supports.  Not just a random attribute.

Either keep this data in private structure or even better, just add the
same thing as you would do with:

 

with all the details of course.  The user can see it, can supply it, old
releases support it, all the stuff is there already.

I'm open to suggestions, but NACK to random "wannabe private" attributes.


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

[libvirt] [PATCH v2] lib: Add note that bulk stats API queries may overrun RPC buffers

[Peter Krempa]

Hint that the users should limit the number of VMs queried in the bulk
stats API.
---
v2:
- added a suggestion of the number of queried VMs (valid after bump to 32M 
message)

 src/libvirt-domain.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 310b91b37..aef061943 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11315,6 +11315,10 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
  * VIR_CONNECT_GET_ALL_DOMAINS_STATS_SHUTOFF and/or
  * VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER for all other states.
  *
+ * Note that this API is prone to exceeding maximum RPC message size on hosts
+ * with lots of VMs so it's suggested to use virDomainListGetStats with a
+ * reasonable list of VMs as the argument.
+ *
  * Returns the count of returned statistics structures on success, -1 on error.
  * The requested data are returned in the @retStats parameter. The returned
  * array should be freed by the caller. See virDomainStatsRecordListFree.
@@ -11386,6 +11390,10 @@ virConnectGetAllDomainStats(virConnectPtr conn,
  * Note that any of the domain list filtering flags in @flags may be rejected
  * by this function.
  *
+ * Note that this API is prone to exceeding maximum RPC if querying too many 
VMs
+ * with lots of statistics. It's suggested to query in batches of 10VMs, which
+ * should be good enough for VMs with 3000 disks + networks.
+ *
  * Returns the count of returned statistics structures on success, -1 on error.
  * The requested data are returned in the @retStats parameter. The returned
  * array should be freed by the caller. See virDomainStatsRecordListFree.
-- 
2.12.2

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

Re: [libvirt] [PATCH 1/2] conf: don't iterate over backcompat console in virDomainChrDefForeach

[Martin Kletzander]

On Mon, May 15, 2017 at 04:28:34PM +0200, Pavel Hrdina wrote:

If the first console is just a copy of the first serial device we
don't need to iterate over the same device twice in order to perform
actions like security labeling, cgroup configuring, etc.

Currently only security SELinux manager was aware of this fact.

Signed-off-by: Pavel Hrdina 
---
src/conf/domain_conf.c  | 26 +-
src/security/security_selinux.c | 10 --
2 files changed, 21 insertions(+), 15 deletions(-)



ACK


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

[libvirt] [PATCH v3 3/3] tests : Testcases for loadparm

[Farhan Ali]

Add testcases for loadparm

Signed-off-by: Farhan Ali 
Reviewed-by: Boris Fiuczynski 
Reviewed-by: Bjoern Walk 
Reviewed-by: Marc Hartmayer 
---
 ...-machine-loadparm-multiple-disks-nets-s390.args | 28 ++
 ...v-machine-loadparm-multiple-disks-nets-s390.xml | 43 ++
 .../qemuxml2argv-machine-loadparm-net-s390.args| 20 ++
 .../qemuxml2argv-machine-loadparm-net-s390.xml | 26 +
 ...xml2argv-machine-loadparm-s390-char-invalid.xml | 26 +
 ...uxml2argv-machine-loadparm-s390-len-invalid.xml | 26 +
 .../qemuxml2argv-machine-loadparm-s390.args| 20 ++
 .../qemuxml2argv-machine-loadparm-s390.xml | 26 +
 tests/qemuxml2argvtest.c   | 19 ++
 9 files changed, 234 insertions(+)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-multiple-disks-nets-s390.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-multiple-disks-nets-s390.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-net-s390.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-net-s390.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390-char-invalid.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390-len-invalid.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390.xml

diff --git 
a/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-multiple-disks-nets-s390.args
 
b/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-multiple-disks-nets-s390.args
new file mode 100644
index 000..c9e9061
--- /dev/null
+++ 
b/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-multiple-disks-nets-s390.args
@@ -0,0 +1,28 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-s390x \
+-name QEMUGuest1 \
+-S \
+-machine s390-ccw-virtio,loadparm=SYSTEM1,accel=tcg \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
+-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-virtio-disk0 \
+-device virtio-blk-ccw,devno=fe.0.0002,drive=drive-virtio-disk0,\
+id=virtio-disk0,bootindex=1 \
+-drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-virtio-disk1 \
+-device virtio-blk-ccw,devno=fe.0.0003,drive=drive-virtio-disk1,\
+id=virtio-disk1,bootindex=3 \
+-device virtio-net-ccw,vlan=0,id=net0,mac=00:11:22:33:44:54,devno=fe.0.,\
+bootindex=2 \
+-net user,vlan=0,name=hostnet0 \
+-device virtio-net-ccw,vlan=1,id=net1,mac=00:11:22:33:42:36,devno=fe.0.0004 \
+-net user,vlan=1,name=hostnet1 \
+-device virtio-balloon-ccw,id=balloon0,devno=fe.0.0001
diff --git 
a/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-multiple-disks-nets-s390.xml
 
b/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-multiple-disks-nets-s390.xml
new file mode 100644
index 000..df442e9
--- /dev/null
+++ 
b/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-multiple-disks-nets-s390.xml
@@ -0,0 +1,43 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219136
+  219136
+  1
+  
+hvm
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-s390x
+
+  
+  
+  
+  
+
+
+  
+  
+  
+  
+
+
+  
+  
+  
+  
+
+   
+  
+  
+  
+
+
+  
+
+  
+
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-net-s390.args 
b/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-net-s390.args
new file mode 100644
index 000..4ca2865
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-net-s390.args
@@ -0,0 +1,20 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-s390x \
+-name QEMUGuest1 \
+-S \
+-machine s390-ccw-virtio,loadparm=2,accel=tcg \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
+-device virtio-net-ccw,vlan=0,id=net0,mac=00:11:22:33:44:54,devno=fe.0.,\
+bootindex=1 \
+-net user,vlan=0,name=hostnet0 \
+-device virtio-balloon-ccw,id=balloon0,devno=fe.0.0001
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-net-s390.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-net-s390.xml
new file mode 100644
index 000..ca682af
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-net-s390.xml
@@ -0,0 +1,26 @@
+
+  QEMUGuest1
+  

[libvirt] [PATCH v3 1/3] conf : Add loadparm boot option for a boot device

[Farhan Ali]

Update the per device boot schema to add an optional loadparm parameter.
Extend the virDomainDeviceInfo to support loadparm option.
Modify the appropriate functions to parse loadparm from boot device xml.

Signed-off-by: Farhan Ali 
Reviewed-by: Bjoern Walk 
Reviewed-by: Boris Fiuczynski 
Reviewed-by: Marc Hartmayer 
---
 docs/formatdomain.html.in |  9 --
 docs/news.xml | 11 +++
 docs/schemas/domaincommon.rng |  7 +
 src/conf/device_conf.h|  1 +
 src/conf/domain_conf.c| 69 +--
 5 files changed, 93 insertions(+), 4 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 3135db4..fd6f666 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3022,8 +3022,13 @@
   boot
   Specifies that the disk is bootable. The order
 attribute determines the order in which devices will be tried during
-boot sequence. The per-device boot elements cannot be
-used together with general boot elements in
+boot sequence. On S390 architecture only the first boot device is used.
+The optional loadparm attribute is an 8 byte parameter
+which can be queried by guests on S390 via sclp or diag 308. Linux
+guests on S390 can use loadparm to select a boot entry.
+Since 3.4.0
+The per-device boot elements cannot be used together
+with general boot elements in
 BIOS bootloader section.
 Since 0.8.8
   
diff --git a/docs/news.xml b/docs/news.xml
index 52915ee..bd9e43a 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -37,6 +37,17 @@
 
   
 
+  qemu: Add support for loadparm for a boot device
+
+
+  Add an optional boot parameter 'loadparm' for a boot device.
+  Loadparm is a 8 byte parameter than can be queried by S390 guests
+  via sclp or diag 308. Linux guests on S390 use it to select a boot
+  entry.
+
+  
+  
+
   Improved streams to efficiently transfer sparseness
 
 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index f88e84a..c885aec 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5026,6 +5026,13 @@
   
 
   
+  
+
+  
+[a-zA-Z0-9.\s]{1,8}
+  
+
+  
   
 
   
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index a20de85..48782be 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -167,6 +167,7 @@ struct _virDomainDeviceInfo {
  * assignment, never saved and never reported.
  */
 int pciConnectFlags; /* enum virDomainPCIConnectFlags */
+char *loadparm;
 };
 
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8c62673..dbf6108 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3520,6 +3520,7 @@ void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info)
 memset(>addr, 0, sizeof(info->addr));
 info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE;
 VIR_FREE(info->romfile);
+VIR_FREE(info->loadparm);
 }
 
 
@@ -5199,6 +5200,48 @@ virDomainDefValidate(virDomainDefPtr def,
 return 0;
 }
 
+/**
+ * virDomainDeviceLoadparmIsValid
+ * @loadparm : The string to validate
+ *
+ * The valid set of values for loadparm are [a-zA-Z0-9.]
+ * and blank spaces.
+ * The maximum allowed length is 8 characters.
+ * An empty string is considered invalid
+ */
+static bool
+virDomainDeviceLoadparmIsValid(const char *loadparm)
+{
+size_t i;
+
+if (virStringIsEmpty(loadparm)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("loadparm cannot be an empty string"));
+return false;
+}
+
+if (strlen(loadparm) > 8) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("loadparm '%s' exceeds 8 characters"), loadparm);
+return false;
+}
+
+for (i = 0; i < strlen(loadparm); i++) {
+uint8_t c = loadparm[i];
+
+if (('A' <= c && c <= 'Z') || ('0' <= c && c <= '9') ||
+(c == '.') || (c == ' ')) {
+continue;
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("invalid loadparm char '%c', expecting chars"
+ " in set of [a-zA-Z0-9.] and blank spaces"), c);
+return false;
+}
+}
+
+return true;
+}
 
 /* Generate a string representation of a device address
  * @info address Device address to stringify
@@ -5208,9 +5251,14 @@ virDomainDeviceInfoFormat(virBufferPtr buf,
   virDomainDeviceInfoPtr info,
   unsigned int flags)
 {
-if ((flags & VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) && info->bootIndex)
- 

[libvirt] [PATCH v3 2/3] qemu : Add loadparm to qemu command line string

[Farhan Ali]

Introduce a new QEMU capability for loadparm and if the capability is
supported by QEMU then append the loadparm value to "-machine" string
of qemu command line.

Signed-off-by: Farhan Ali 
Reviewed-by: Bjoern Walk 
Reviewed-by: Boris Fiuczynski 
Reviewed-by: Marc Hartmayer 
---
 src/qemu/qemu_capabilities.c |  2 ++
 src/qemu/qemu_capabilities.h |  1 +
 src/qemu/qemu_command.c  | 37 +
 3 files changed, 40 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 546dfd7..e3bd445 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -371,6 +371,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "kernel-irqchip.split",
   "intel-iommu.intremap",
   "intel-iommu.caching-mode",
+  "loadparm",
 );
 
 
@@ -3144,6 +3145,7 @@ static struct virQEMUCapsCommandLineProps 
virQEMUCapsCommandLine[] = {
 { "drive", "throttling.group", QEMU_CAPS_DRIVE_IOTUNE_GROUP },
 { "spice", "rendernode", QEMU_CAPS_SPICE_RENDERNODE },
 { "machine", "kernel_irqchip", QEMU_CAPS_MACHINE_KERNEL_IRQCHIP },
+{ "machine", "loadparm", QEMU_CAPS_LOADPARM },
 };
 
 static int
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index aa99fda..a18c61c 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -409,6 +409,7 @@ typedef enum {
 QEMU_CAPS_MACHINE_KERNEL_IRQCHIP_SPLIT, /* -machine kernel_irqchip=split */
 QEMU_CAPS_INTEL_IOMMU_INTREMAP, /* intel-iommu.intremap */
 QEMU_CAPS_INTEL_IOMMU_CACHING_MODE, /* intel-iommu.caching-mode */
+QEMU_CAPS_LOADPARM, /* -machine loadparm */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index aa66e3d..1291b62 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7226,6 +7226,41 @@ qemuAppendKeyWrapMachineParms(virBuffer *buf, 
virQEMUCapsPtr qemuCaps,
 return true;
 }
 
+static void
+qemuAppendLoadparmMachineParm(virBuffer *buf,
+  const virDomainDef *def,
+  virQEMUCapsPtr qemuCaps)
+{
+size_t i = 0;
+
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX) ||
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_LOADPARM))
+return;
+
+for (i = 0; i < def->ndisks; i++) {
+virDomainDiskDefPtr disk = def->disks[i];
+
+if (disk->info.bootIndex == 1) {
+if (disk->info.loadparm)
+virBufferAsprintf(buf, ",loadparm=%s", disk->info.loadparm);
+
+return;
+}
+}
+
+/* Network boot device*/
+for (i = 0; i < def->nnets; i++) {
+virDomainNetDefPtr net = def->nets[i];
+
+if (net->info.bootIndex == 1) {
+if (net->info.loadparm)
+virBufferAsprintf(buf, ",loadparm=%s", net->info.loadparm);
+
+return;
+}
+}
+}
+
 static int
 qemuBuildNameCommandLine(virCommandPtr cmd,
  virQEMUDriverConfigPtr cfg,
@@ -7315,6 +7350,8 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
 virCommandAddArg(cmd, "-machine");
 virBufferAdd(, def->os.machine, -1);
 
+qemuAppendLoadparmMachineParm(, def, qemuCaps);
+
 if (def->virtType == VIR_DOMAIN_VIRT_QEMU)
 virBufferAddLit(, ",accel=tcg");
 else if (def->virtType == VIR_DOMAIN_VIRT_KVM)
-- 
1.9.1

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

[libvirt] [PATCH v3 0/3] Loadparm support

[Farhan Ali]

This patch series introduces the support for new s390x 'loadparm'
feature. The 'loadparm' can be used to select the boot entry to
boot from, for a boot device.

Here is a link to the QEMU patches:
https://lists.nongnu.org/archive/html/qemu-devel/2017-05/msg00192.html

ChangeLog
-
v2 -> v3:
- Updated news.xml and formatdomain.html.in with a more architectural
description of loadparm (patch 1)

v1 -> v2:
- Rebased the patch series on the latest master, commit
 2f69dd3 virfiletest: include linux/falloc.h

Thanks
Farhan Ali

Farhan Ali (3):
  conf : Add loadparm boot option for a boot device
  qemu : Add loadparm to qemu command line string
  tests : Testcases for loadparm

 docs/formatdomain.html.in  |  9 ++-
 docs/news.xml  | 11 
 docs/schemas/domaincommon.rng  |  7 +++
 src/conf/device_conf.h |  1 +
 src/conf/domain_conf.c | 69 +-
 src/qemu/qemu_capabilities.c   |  2 +
 src/qemu/qemu_capabilities.h   |  1 +
 src/qemu/qemu_command.c| 37 
 ...-machine-loadparm-multiple-disks-nets-s390.args | 28 +
 ...v-machine-loadparm-multiple-disks-nets-s390.xml | 43 ++
 .../qemuxml2argv-machine-loadparm-net-s390.args| 20 +++
 .../qemuxml2argv-machine-loadparm-net-s390.xml | 26 
 ...xml2argv-machine-loadparm-s390-char-invalid.xml | 26 
 ...uxml2argv-machine-loadparm-s390-len-invalid.xml | 26 
 .../qemuxml2argv-machine-loadparm-s390.args| 20 +++
 .../qemuxml2argv-machine-loadparm-s390.xml | 26 
 tests/qemuxml2argvtest.c   | 19 ++
 17 files changed, 367 insertions(+), 4 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-multiple-disks-nets-s390.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-multiple-disks-nets-s390.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-net-s390.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-net-s390.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390-char-invalid.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390-len-invalid.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-machine-loadparm-s390.xml

-- 
1.9.1

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

Re: [libvirt] [libvirt-python][PATCH 0/4] Implement sparse streams

[Martin Kletzander]

On Mon, May 22, 2017 at 03:33:21PM +0200, Michal Privoznik wrote:

On 05/22/2017 12:57 PM, Michal Privoznik wrote:

*** BLURB HERE ***

Michal Privoznik (4):
  Implement virStreamSendHole/virStreamRecvHole
  virStream: Introduce virStreamRecvFlags
  virStream: Introduce virStreamSparse{Recv,Send}All
  examples: Introduce sparsestream.py

 examples/sparsestream.py  | 119 
 generator.py  |   5 ++
 libvirt-override-virStream.py | 156 ++
 libvirt-override.c| 102 +++
 4 files changed, 382 insertions(+)
 create mode 100755 examples/sparsestream.py



D'oh! I haven't had nose installed. Consider this squashed into respective 
commits:



And I considered this squashed in to them when doing the review.


diff --git i/sanitytest.py w/sanitytest.py
index 7183baa..deec200 100644
--- i/sanitytest.py
+++ w/sanitytest.py
@@ -167,7 +167,8 @@ for cname in wantfunctions:
# These aren't functions, they're callback signatures
if name in ["virConnectAuthCallbackPtr", "virConnectCloseFunc",
"virStreamSinkFunc", "virStreamSourceFunc", 
"virStreamEventCallback",
-"virEventHandleCallback", "virEventTimeoutCallback", 
"virFreeCallback"]:
+"virEventHandleCallback", "virEventTimeoutCallback", 
"virFreeCallback",
+"virStreamSinkHoleFunc", "virStreamSourceHoleFunc", 
"virStreamSourceSkipFunc"]:
continue
if name[0:21] == "virConnectDomainEvent" and name[-8:] == "Callback":
continue
@@ -373,7 +374,8 @@ for name in sorted(finalklassmap):

# These exist in C and exist in python, but we've got
# a pure-python impl so don't check them
-if name in ["virStreamRecvAll", "virStreamSendAll"]:
+if name in ["virStreamRecvAll", "virStreamSendAll",
+"virStreamSparseRecvAll", "virStreamSparseSendAll"]:
continue

try:

Michal

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


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

Re: [libvirt] [libvirt-python][PATCH 4/4] examples: Introduce sparsestream.py

[Martin Kletzander]

On Mon, May 22, 2017 at 12:57:15PM +0200, Michal Privoznik wrote:

Sparse streams are not that straight forward to use for the very
first time. Especially the sparseRecvAll() and sparseSendAll()
methods which expects callbacks. What we can do to make it easier
for developers is to have an example where they can take an
inspiration from.

Signed-off-by: Michal Privoznik 
---
examples/sparsestream.py | 119 +++
1 file changed, 119 insertions(+)
create mode 100755 examples/sparsestream.py

diff --git a/examples/sparsestream.py b/examples/sparsestream.py
new file mode 100755
index 000..b572a31
--- /dev/null
+++ b/examples/sparsestream.py
@@ -0,0 +1,119 @@
+#!/usr/bin/env python
+# Example of sparse streams usage
+#
+# Authors:
+#   Michal Privoznik 
+


Don't worry, we can handle Unicode.


+import libvirt, sys, os
+
+def bytesWriteHandler(stream, buf, opaque):
+fd = opaque


Kind of pointless line, but I guess you wanted to show a point.


+return os.write(fd, buf)
+
+def bytesReadHandler(stream, nbytes, opaque):
+fd = opaque
+return os.read(fd, nbytes)
+
+def recvSkipHandler(stream, length, opaque):
+fd = opaque
+cur = os.lseek(fd, length, os.SEEK_CUR)
+return os.ftruncate(fd, cur)
+
+def sendSkipHandler(stream, length, opaque):
+fd = opaque
+return os.lseek(fd, length, os.SEEK_CUR)
+
+def holeHandler(stream, opaque):
+fd = opaque
+cur = os.lseek(fd, 0, os.SEEK_CUR)
+
+try:
+try:
+data = os.lseek(fd, cur, os.SEEK_DATA)


The only way I saw this raise an exception is when you are exactly at
the end of the file.  That's something that can be checked for *and* it
raises precisely OSError(6).


+# There are three options:
+# 1) data == cur;  @cur is in data
+# 2) data > cur; @cur is in a hole, next data at @data
+# 3) data < 0; either @cur is in trailing hole, or @cur is beyond 
EOF.
+except:
+# case 3
+inData = False
+eof = os.lseek(fd, 0, os.SEEK_END)
+if (eof < cur):
+raise RuntimeError("Current position in file after EOF: %d" % 
cur)
+sectionLen = eof - cur
+else:
+if (data > cur):
+# case 2
+inData = False
+sectionLen = data - cur
+else:
+# case 1
+inData = True
+
+# We don't know where does the next hole start. Let's find out.
+# Here we get the same options as above
+try:
+hole = os.lseek(fd, data, os.SEEK_HOLE)
+except:
+# case 3. But wait a second. There is always a trailing 
hole.
+# Do the best what we can here
+raise RuntimeError("No trailing hole")
+
+if (hole == data):
+# case 1. Again, this is suspicious. The reason we are 
here is
+# because we are in data. But at the same time we are in a
+# hole. WAT?
+raise RuntimeError("Impossible happened")
+else:
+# case 2
+sectionLen = hole - data
+finally:
+os.lseek(fd, cur, os.SEEK_SET)


I would drop the exception handlers (maybe keep the one for that one
os.lseek) above there.  What is the point in seeking to the previous
position when something is wrong and the program is ending anyway.


+return [inData, sectionLen]
+
+def download(vol, st, filename):
+offset = 0
+length = 0
+
+try:
+fd = os.open(filename, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 
mode=0o0660)
+vol.download(st, offset, length, 
libvirt.VIR_STORAGE_VOL_DOWNLOAD_SPARSE_STREAM)
+st.sparseRecvAll(bytesWriteHandler, recvSkipHandler, fd)
+except OSError:
+print(sys.exc_info()[0])


Again, 'OSError as e', but I would not except such program to cleanup
after itself (e.g. I would like to, for example, kill it and see the
part that was output in the file, like you would do with 'cp', 'cat',
etc.)  Not only will it clean up the code, but it will also behave as
other programs (i.e. as expected).


+os.unlink(filename)
+
+os.close(fd)
+


What does closing after unlink do on windows? :D

Other than that it looks okay.


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

Re: [libvirt] [libvirt-python][PATCH 3/4] virStream: Introduce virStreamSparse{Recv, Send}All

[Martin Kletzander]

On Mon, May 22, 2017 at 12:57:14PM +0200, Michal Privoznik wrote:

Yet again, our parser is not capable of generating proper
wrapper. To be fair, this one wold be really tough anyway.

Signed-off-by: Michal Privoznik 
---
generator.py  |   2 +
libvirt-override-virStream.py | 117 ++
2 files changed, 119 insertions(+)

diff --git a/generator.py b/generator.py
index 0e07fc8..93d1dc3 100755
--- a/generator.py
+++ b/generator.py
@@ -546,6 +546,8 @@ skip_function = (
'virStreamRecvHole', # overridden in libvirt-override-virStream.py
'virStreamSendHole', # overridden in libvirt-override-virStream.py
'virStreamRecvFlags', # overridden in libvirt-override-virStream.py
+'virStreamSparseRecvAll', # overridden in libvirt-override-virStream.py
+'virStreamSparseSendAll', # overridden in libvirt-override-virStream.py

'virConnectUnregisterCloseCallback', # overridden in virConnect.py
'virConnectRegisterCloseCallback', # overridden in virConnect.py
diff --git a/libvirt-override-virStream.py b/libvirt-override-virStream.py
index 66d2bf6..568bd9f 100644
--- a/libvirt-override-virStream.py
+++ b/libvirt-override-virStream.py
@@ -164,3 +164,120 @@
ret = libvirtmod.virStreamRecvFlags(self._o, nbytes, flags)
if ret is None: raise libvirtError ('virStreamRecvFlags() failed')
return ret
+
+def sparseRecvAll(self, handler, holeHandler, opaque):
+"""Receive the entire data stream, sending the data to
+the requested data sink handler and calling the skip
+holeHandler to generate holes for sparse stream targets.
+This is simply a convenient alternative to recvFlags, for
+apps that do blocking-I/O.
+


I would add "And want to preserve sparseness."


+Hypothetical callbacks can look like this:
+
+def handler(stream, # virStream instance
+buf,# string containing received data
+opaque): # extra data passed to sparseRecvAll as opaque
+fd = opaque
+return os.write(fd, buf)
+


So this is the same handler example as in recvAll(), but ... [1]


+def holeHandler(stream, # virStream instance
+length, # number of bytes to skip
+opaque): # extra data passed to sparseRecvAll as 
opaque
+fd = opaque
+cur = os.lseek(fd, length, os.SEEK_CUR)
+return os.ftruncate(fd, cur) # take this extra step to
+ # actually allocate the hole
+"""
+while True:
+want = 64 * 1024
+got = self.recvFlags(want, VIR_STREAM_RECV_STOP_AT_HOLE)
+if got == -2:
+raise libvirtError("cannot use sparseRecvAll with "
+   "nonblocking stream")
+if got == -3:
+length = self.recvHole()
+if (length is None):


No need for the parentheses.


+self.abort()
+raise RuntimeError("recvHole handler failed")
+try:
+   ret = holeHandler(self, length, opaque)


[1] ... you are handling the return value (or exception in this case)
differently.  I think you should check for the return value and change
the example.  The reasoning behind that is that you can get so many
exceptions that you can't easily differentiate between those you want
and those you don't.  Moreover you are catching *all* exceptions, which
you should not do.  In some versions you can get InterruptedError, but
not in newer ones, you can get KeyboardInterrupt, and it should be the
handler's job to decide what to do.


+except:
+self.abort()
+continue
+
+if len(got) == 0:
+break
+
+try:
+ret = handler(self, got, opaque)
+if type(ret) is int and ret < 0:
+raise RuntimeError("recvAll handler returned %d" % ret)


See, here you check for the return value.


+except Exception:
+e = sys.exc_info()[1]


You can just do:
except Exception as e:


+try:
+self.abort()
+except:
+pass
+raise e


And now you're even wrapping the abort() in try/except, but you weren't
before.  I would say that if there's an exception from almost anywhere,
we should just not catch it because most of the decisions on what to
catch will be wrong in some situation.

same for SendAll.


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

Re: [libvirt] [libvirt-python][PATCH 1/4] Implement virStreamSendHole/virStreamRecvHole

[Martin Kletzander]

On Tue, May 23, 2017 at 12:52:34PM +0200, Martin Kletzander wrote:

On Mon, May 22, 2017 at 12:57:12PM +0200, Michal Privoznik wrote:

The return value for virStreamRecvHole is slightly different to
its C counterpart. In python, either it returns the hole size or
None if C API fails.

Signed-off-by: Michal Privoznik 
---
generator.py  |  2 ++
libvirt-override-virStream.py | 21 +++
libvirt-override.c| 63 +++
3 files changed, 86 insertions(+)

diff --git a/libvirt-override.c b/libvirt-override.c
index a762941..b5e11ed 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -9463,6 +9463,65 @@ libvirt_virConnectSecretEventDeregisterAny(PyObject 
*self ATTRIBUTE_UNUSED,
}
#endif /* LIBVIR_CHECK_VERSION(3, 0, 0)*/

+
+#if LIBVIR_CHECK_VERSION(3, 4, 0)
+static PyObject *
+libvirt_virStreamRecvHole(PyObject *self ATTRIBUTE_UNUSED,
+  PyObject *args)
+{
+PyObject *pyobj_stream;
+virStreamPtr stream;
+long long length = -1;
+unsigned int flags;
+int ret;
+
+if (!PyArg_ParseTuple(args, (char *) "OI:virStreamRecvHole",
+  _stream, ))
+return NULL;
+
+stream = PyvirStream_Get(pyobj_stream);
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+ret = virStreamRecvHole(stream, , flags);
+LIBVIRT_END_ALLOW_THREADS;
+
+DEBUG("StreamRecvHole ret=%d length=%lld\n", ret, length);
+
+if (ret < 0)
+return VIR_PY_NONE;
+
+return libvirt_longlongWrap(length);
+}
+
+
+static PyObject *
+libvirt_virStreamSendHole(PyObject *self ATTRIBUTE_UNUSED,
+  PyObject *args)
+{
+PyObject *pyobj_stream;
+virStreamPtr stream;
+long long length;
+unsigned int flags;
+int ret;
+
+if (!PyArg_ParseTuple(args, (char *) "OKI:virStreamSendHole",


s/OKI/OLI/, I believe


Oh, and ACK of course.


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

Re: [libvirt] [libvirt-python][PATCH 2/4] virStream: Introduce virStreamRecvFlags

[Martin Kletzander]

On Mon, May 22, 2017 at 12:57:13PM +0200, Michal Privoznik wrote:

Yet again, we need a custom wrapper over virStreamRecvFlags
because our generator is not capable of generating it.

Signed-off-by: Michal Privoznik 
---
generator.py  |  1 +
libvirt-override-virStream.py | 18 ++
libvirt-override.c| 39 +++
3 files changed, 58 insertions(+)



ACK


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

Re: [libvirt] [libvirt-python][PATCH 1/4] Implement virStreamSendHole/virStreamRecvHole

[Martin Kletzander]

On Mon, May 22, 2017 at 12:57:12PM +0200, Michal Privoznik wrote:

The return value for virStreamRecvHole is slightly different to
its C counterpart. In python, either it returns the hole size or
None if C API fails.

Signed-off-by: Michal Privoznik 
---
generator.py  |  2 ++
libvirt-override-virStream.py | 21 +++
libvirt-override.c| 63 +++
3 files changed, 86 insertions(+)

diff --git a/libvirt-override.c b/libvirt-override.c
index a762941..b5e11ed 100644
--- a/libvirt-override.c
+++ b/libvirt-override.c
@@ -9463,6 +9463,65 @@ libvirt_virConnectSecretEventDeregisterAny(PyObject 
*self ATTRIBUTE_UNUSED,
}
#endif /* LIBVIR_CHECK_VERSION(3, 0, 0)*/

+
+#if LIBVIR_CHECK_VERSION(3, 4, 0)
+static PyObject *
+libvirt_virStreamRecvHole(PyObject *self ATTRIBUTE_UNUSED,
+  PyObject *args)
+{
+PyObject *pyobj_stream;
+virStreamPtr stream;
+long long length = -1;
+unsigned int flags;
+int ret;
+
+if (!PyArg_ParseTuple(args, (char *) "OI:virStreamRecvHole",
+  _stream, ))
+return NULL;
+
+stream = PyvirStream_Get(pyobj_stream);
+
+LIBVIRT_BEGIN_ALLOW_THREADS;
+ret = virStreamRecvHole(stream, , flags);
+LIBVIRT_END_ALLOW_THREADS;
+
+DEBUG("StreamRecvHole ret=%d length=%lld\n", ret, length);
+
+if (ret < 0)
+return VIR_PY_NONE;
+
+return libvirt_longlongWrap(length);
+}
+
+
+static PyObject *
+libvirt_virStreamSendHole(PyObject *self ATTRIBUTE_UNUSED,
+  PyObject *args)
+{
+PyObject *pyobj_stream;
+virStreamPtr stream;
+long long length;
+unsigned int flags;
+int ret;
+
+if (!PyArg_ParseTuple(args, (char *) "OKI:virStreamSendHole",


s/OKI/OLI/, I believe


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

Re: [libvirt] [RFC PATCH 2/2] lib: Add note that bulk stats API queries may overrun RPC buffers

[Peter Krempa]

On Tue, May 23, 2017 at 10:04:44 +0100, Daniel Berrange wrote:
> On Tue, May 23, 2017 at 10:51:47AM +0200, Martin Kletzander wrote:
> > On Tue, May 23, 2017 at 09:32:03AM +0100, Daniel P. Berrange wrote:
> > > On Mon, May 22, 2017 at 06:00:13PM +0200, Peter Krempa wrote:
> > > > Hint that the users should limit the number of VMs queried in the bulk
> > > > stats API.

[...]

> > Or we can give a hint on what the limit is and let users figure out the
> > sensible number.  It is not only based on the number of VMs.  I believe
> > you can hit the limit with one or two VMs if you have lot of devices
> > that we report statistics for.  Limiting the number of VMs to a
> > particular number would not help as much in this case.  But we can
> > combine both approaches.
> 
> Urgh, that's even worse. Apps can't simply split queries into blocks of
> N vms, because any single VM in that list might have huge number of disks.
> So to use this at all reliably you have to query the XML config of every
> guest to see what devices are present and then split up the queries into
> variable number of VMs :-(
> 
> This just adds to my feeling that we should consider this API a failed
> experiment

Given this metric, any API in libvirt that takes XML is failed in the
same sense. You can easily have a VM that exceeds the 4MiB string RPC
limit (and 16, or 32) MiB in this sense anyways.

Along with metadata, you can reach the limit even easier.

A disk information dump returned with this API has slightly above 600
bytes, so let's say 1k, with this metric you can have a VM with 16k
disks/nics/whatever, which is reasonable. With the bump to 32MiB, you
can obviously have even more.

Since this API makes sense (saves time) even if called for a singe VM I
don't really think this is a big problem. If you say that the number of
VMs you should query is let's say 2 or 4, you can have giant guests.

The only non-scalable part is if you have lots of giant guests. You
can't really fix that.


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

Re: [libvirt] [RFC PATCH 2/2] lib: Add note that bulk stats API queries may overrun RPC buffers

[Martin Kletzander]

On Tue, May 23, 2017 at 10:04:44AM +0100, Daniel P. Berrange wrote:

On Tue, May 23, 2017 at 10:51:47AM +0200, Martin Kletzander wrote:

On Tue, May 23, 2017 at 09:32:03AM +0100, Daniel P. Berrange wrote:
> On Mon, May 22, 2017 at 06:00:13PM +0200, Peter Krempa wrote:
> > Hint that the users should limit the number of VMs queried in the bulk
> > stats API.
> > ---
> >  src/libvirt-domain.c | 7 +++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> > index 310b91b37..b01f2705c 100644
> > --- a/src/libvirt-domain.c
> > +++ b/src/libvirt-domain.c
> > @@ -11315,6 +11315,10 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
> >   * VIR_CONNECT_GET_ALL_DOMAINS_STATS_SHUTOFF and/or
> >   * VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER for all other states.
> >   *
> > + * Note that this API is prone to exceeding maximum RPC message size on 
hosts
> > + * with lots of VMs so it's suggested to use virDomainListGetStats with a
> > + * reasonable list of VMs as the argument.
> > + *
> >   * Returns the count of returned statistics structures on success, -1 on 
error.
> >   * The requested data are returned in the @retStats parameter. The returned
> >   * array should be freed by the caller. See virDomainStatsRecordListFree.
> > @@ -11386,6 +11390,9 @@ virConnectGetAllDomainStats(virConnectPtr conn,
> >   * Note that any of the domain list filtering flags in @flags may be 
rejected
> >   * by this function.
> >   *
> > + * Note that this API is prone to exceeding maximum RPC message size on 
hosts
> > + * with lots of VMs so it's suggested to limit the number of VMs queried.
>
> With the way this is worded, applications have no guidance on what is a
> sensible max number of VMs they can safely use, so I don't think it is
> particularly useful, except as a way to say "we told you so" when apps
> report a bug.
>
> I'd be inclined to explicitly put a limit of 100 VMs[1] in the API, and
> document that hard limit, so apps immediately know to write their code
> to chunk in 100 VM blocks.
>

Or we can give a hint on what the limit is and let users figure out the
sensible number.  It is not only based on the number of VMs.  I believe
you can hit the limit with one or two VMs if you have lot of devices
that we report statistics for.  Limiting the number of VMs to a
particular number would not help as much in this case.  But we can
combine both approaches.


Urgh, that's even worse. Apps can't simply split queries into blocks of
N vms, because any single VM in that list might have huge number of disks.
So to use this at all reliably you have to query the XML config of every
guest to see what devices are present and then split up the queries into
variable number of VMs :-(



I meant something along the lines of:

 This API might transfer lot of data over the connection which needs to
 be limited.  The limit is currently 32MB with all the headers and the
 output format is described here, so one should be able to make a
 pretty good assumptions based on the average deployment they have.  In
 case you get error message saying "RPC: message to big", you need to
 limit the number of VMs you are requesting statistics for.

And then put a limit in the API that returns additional error if more
than 100 VMs are in the list.


This just adds to my feeling that we should consider this API a failed
experiment



I'm not saying no...  But we should be able to do such things, just in a
different way, let's say.  The idea with shared memory is nice.  Even
remotely the libvirt API could expose it as such and underneath we could
be changing how that update routine works.


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


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

Re: [libvirt] [RFC PATCH 2/2] lib: Add note that bulk stats API queries may overrun RPC buffers

[Daniel P. Berrange]

On Tue, May 23, 2017 at 10:51:47AM +0200, Martin Kletzander wrote:
> On Tue, May 23, 2017 at 09:32:03AM +0100, Daniel P. Berrange wrote:
> > On Mon, May 22, 2017 at 06:00:13PM +0200, Peter Krempa wrote:
> > > Hint that the users should limit the number of VMs queried in the bulk
> > > stats API.
> > > ---
> > >  src/libvirt-domain.c | 7 +++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> > > index 310b91b37..b01f2705c 100644
> > > --- a/src/libvirt-domain.c
> > > +++ b/src/libvirt-domain.c
> > > @@ -11315,6 +11315,10 @@ virConnectGetDomainCapabilities(virConnectPtr 
> > > conn,
> > >   * VIR_CONNECT_GET_ALL_DOMAINS_STATS_SHUTOFF and/or
> > >   * VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER for all other states.
> > >   *
> > > + * Note that this API is prone to exceeding maximum RPC message size on 
> > > hosts
> > > + * with lots of VMs so it's suggested to use virDomainListGetStats with a
> > > + * reasonable list of VMs as the argument.
> > > + *
> > >   * Returns the count of returned statistics structures on success, -1 on 
> > > error.
> > >   * The requested data are returned in the @retStats parameter. The 
> > > returned
> > >   * array should be freed by the caller. See virDomainStatsRecordListFree.
> > > @@ -11386,6 +11390,9 @@ virConnectGetAllDomainStats(virConnectPtr conn,
> > >   * Note that any of the domain list filtering flags in @flags may be 
> > > rejected
> > >   * by this function.
> > >   *
> > > + * Note that this API is prone to exceeding maximum RPC message size on 
> > > hosts
> > > + * with lots of VMs so it's suggested to limit the number of VMs queried.
> > 
> > With the way this is worded, applications have no guidance on what is a
> > sensible max number of VMs they can safely use, so I don't think it is
> > particularly useful, except as a way to say "we told you so" when apps
> > report a bug.
> > 
> > I'd be inclined to explicitly put a limit of 100 VMs[1] in the API, and
> > document that hard limit, so apps immediately know to write their code
> > to chunk in 100 VM blocks.
> > 
> 
> Or we can give a hint on what the limit is and let users figure out the
> sensible number.  It is not only based on the number of VMs.  I believe
> you can hit the limit with one or two VMs if you have lot of devices
> that we report statistics for.  Limiting the number of VMs to a
> particular number would not help as much in this case.  But we can
> combine both approaches.

Urgh, that's even worse. Apps can't simply split queries into blocks of
N vms, because any single VM in that list might have huge number of disks.
So to use this at all reliably you have to query the XML config of every
guest to see what devices are present and then split up the queries into
variable number of VMs :-(

This just adds to my feeling that we should consider this API a failed
experiment

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] [RFC PATCH 2/2] lib: Add note that bulk stats API queries may overrun RPC buffers

[Martin Kletzander]

On Tue, May 23, 2017 at 09:32:03AM +0100, Daniel P. Berrange wrote:

On Mon, May 22, 2017 at 06:00:13PM +0200, Peter Krempa wrote:

Hint that the users should limit the number of VMs queried in the bulk
stats API.
---
 src/libvirt-domain.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 310b91b37..b01f2705c 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11315,6 +11315,10 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
  * VIR_CONNECT_GET_ALL_DOMAINS_STATS_SHUTOFF and/or
  * VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER for all other states.
  *
+ * Note that this API is prone to exceeding maximum RPC message size on hosts
+ * with lots of VMs so it's suggested to use virDomainListGetStats with a
+ * reasonable list of VMs as the argument.
+ *
  * Returns the count of returned statistics structures on success, -1 on error.
  * The requested data are returned in the @retStats parameter. The returned
  * array should be freed by the caller. See virDomainStatsRecordListFree.
@@ -11386,6 +11390,9 @@ virConnectGetAllDomainStats(virConnectPtr conn,
  * Note that any of the domain list filtering flags in @flags may be rejected
  * by this function.
  *
+ * Note that this API is prone to exceeding maximum RPC message size on hosts
+ * with lots of VMs so it's suggested to limit the number of VMs queried.


With the way this is worded, applications have no guidance on what is a
sensible max number of VMs they can safely use, so I don't think it is
particularly useful, except as a way to say "we told you so" when apps
report a bug.

I'd be inclined to explicitly put a limit of 100 VMs[1] in the API, and
document that hard limit, so apps immediately know to write their code
to chunk in 100 VM blocks.



Or we can give a hint on what the limit is and let users figure out the
sensible number.  It is not only based on the number of VMs.  I believe
you can hit the limit with one or two VMs if you have lot of devices
that we report statistics for.  Limiting the number of VMs to a
particular number would not help as much in this case.  But we can
combine both approaches.


Regards,
Daniel

[1] 100 is entirely arbitrary for this email - we'd actually pick a number
   based on what we reasonably expect to fit in the RPC message size.
--
|: 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


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

Re: [libvirt] [RFC PATCH 2/2] lib: Add note that bulk stats API queries may overrun RPC buffers

[Daniel P. Berrange]

On Mon, May 22, 2017 at 06:00:13PM +0200, Peter Krempa wrote:
> Hint that the users should limit the number of VMs queried in the bulk
> stats API.
> ---
>  src/libvirt-domain.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 310b91b37..b01f2705c 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -11315,6 +11315,10 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
>   * VIR_CONNECT_GET_ALL_DOMAINS_STATS_SHUTOFF and/or
>   * VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER for all other states.
>   *
> + * Note that this API is prone to exceeding maximum RPC message size on hosts
> + * with lots of VMs so it's suggested to use virDomainListGetStats with a
> + * reasonable list of VMs as the argument.
> + *
>   * Returns the count of returned statistics structures on success, -1 on 
> error.
>   * The requested data are returned in the @retStats parameter. The returned
>   * array should be freed by the caller. See virDomainStatsRecordListFree.
> @@ -11386,6 +11390,9 @@ virConnectGetAllDomainStats(virConnectPtr conn,
>   * Note that any of the domain list filtering flags in @flags may be rejected
>   * by this function.
>   *
> + * Note that this API is prone to exceeding maximum RPC message size on hosts
> + * with lots of VMs so it's suggested to limit the number of VMs queried.

With the way this is worded, applications have no guidance on what is a
sensible max number of VMs they can safely use, so I don't think it is
particularly useful, except as a way to say "we told you so" when apps
report a bug.

I'd be inclined to explicitly put a limit of 100 VMs[1] in the API, and
document that hard limit, so apps immediately know to write their code
to chunk in 100 VM blocks.

Regards,
Daniel

[1] 100 is entirely arbitrary for this email - we'd actually pick a number
based on what we reasonably expect to fit in the RPC message size.
-- 
|: 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 05/13] nodedev: Use common naming for virnodedeviceobj

[Bjoern Walk]

John Ferlan  [2017-05-19, 09:08AM -0400]:

A virNodeDeviceObjPtr is an @obj

A virNodeDeviceObjListPtr is an @devs



More intuitive for the virNodeDeviceObjListPtr would be @objs, but I
guess the naming scheme is already well defined here.


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

Re: [libvirt] [RFC PATCH 2/2] lib: Add note that bulk stats API queries may overrun RPC buffers

[Daniel P. Berrange]

On Tue, May 23, 2017 at 10:12:06AM +0200, Martin Kletzander wrote:
> On Mon, May 22, 2017 at 06:00:13PM +0200, Peter Krempa wrote:
> > Hint that the users should limit the number of VMs queried in the bulk
> > stats API.
> > ---
> > src/libvirt-domain.c | 7 +++
> > 1 file changed, 7 insertions(+)
> > 
> > diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> > index 310b91b37..b01f2705c 100644
> > --- a/src/libvirt-domain.c
> > +++ b/src/libvirt-domain.c
> > @@ -11315,6 +11315,10 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
> >  * VIR_CONNECT_GET_ALL_DOMAINS_STATS_SHUTOFF and/or
> >  * VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER for all other states.
> >  *
> > + * Note that this API is prone to exceeding maximum RPC message size on 
> > hosts
> > + * with lots of VMs so it's suggested to use virDomainListGetStats with a
> > + * reasonable list of VMs as the argument.
> > + *
> >  * Returns the count of returned statistics structures on success, -1 on 
> > error.
> >  * The requested data are returned in the @retStats parameter. The returned
> >  * array should be freed by the caller. See virDomainStatsRecordListFree.
> > @@ -11386,6 +11390,9 @@ virConnectGetAllDomainStats(virConnectPtr conn,
> >  * Note that any of the domain list filtering flags in @flags may be 
> > rejected
> >  * by this function.
> >  *
> > + * Note that this API is prone to exceeding maximum RPC message size on 
> > hosts
> > + * with lots of VMs so it's suggested to limit the number of VMs queried.
> > + *
> 
> Makes sense to me, now we can at least point people somewhere when this
> happens again.  ACK from me, but feel free to wait if you want to really
> make this an RFC and you want more opinions.
> 
> Is there a discussion about how to continue for the future?  I remember
> Michal starting some discussion, but I'm not sure whether that was about
> the same thing.  Anyway, that's just a rhetorical question so that I can
> say the two ideas I have:
> 
> 1) Just increase the limit over time.  Computers and networks are
>getting faster, there's more storage space and memory, and so on.
>It only makes sense to do scale other things respectively.
> 
> 2) Have a new API that streams the data back over virStream.  We can
>then do it continuously (like every X seconds), that might help
>management apps as well because I suspect that's how they use this
>API anyway.
> 
> Thanks for listening ;)

Before we do either of those we should consider just changing the RPC
message format for the APIs in question.

Essentially have, say, 10 statistics we are grabbing for every VM.
On the wire we are encoding


  : 
  : 
  ...
  : 

  : 
  : 
  ...
  : 

  : 
  : 
  ...
  : 

where  is a long string that is basically the same for
every VM, while  is just an int64.

We could change this so that we have a table of names at the
start

  : 
  : 
  ...
  : 

and then for each VM we have

  : 
  : 
  ...
  : 

Soo the name string is no longer repeated, and in its place is an
integer index. If every name is say 12 characters long, and the
index is a 4 byte int, we've reduced a 20 byte packet per stat
to 12 bytes.

That's just one idea, possibly/probably not even the best - there's
various other ways we could encode the data to make it more efficient.

We could perhaps even consider changing the public API too, because
the typed parameters are a really inefficient way to exporting the
data in the API too


If you want to get more radical with a push based solution, then I
would suggest we consider setting up shared memory segment between
libvirt & the client app where we just continuously update the data,
avoiding RPC entirely.

This would only work for apps running locally of course, but at a
large scale, it seems the major apps using libvirt have all taken
the approach of having a local libvirtd connection, rather than
trying to use our remote RPC at data center scale levels from
a central host.

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 00/13] Make the virNodeDeviceObjPtr private

[Bjoern Walk]

John Ferlan  [2017-05-19, 09:08AM -0400]:

All part of the effort I have to have a common object model. This series
is node device test, driver, and virnodedevobj related.

There's also a couple of bug fixes at the beginning of the series from
things I have found during this effort.

There's still a few more patches in local branches to make the
virNodeDeviceObjListPtr private as well, but those have some merge
needs with other patches currently on list elsewhere, so I'll hold
onto them for now.



Am I missing something or where does the privatization actually happen?

In general, a lot of the cleanups you are performing are beneficial for
the readability. My biggest concern is that this will generate a lot of
work when backporting patches.


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

Re: [libvirt] [PATCH 04/13] nodedev: Move node_device_linux_sysfs from node_driver to conf

[Bjoern Walk]

John Ferlan  [2017-05-19, 09:08AM -0400]:

Move the whole file from src/node_device into src/conf and rename the
API's to have the "virNodeDevice" prefix.

Signed-off-by: John Ferlan 
---
src/Makefile.am|  8 
.../node_device_linux_sysfs.c  | 24 ++
.../node_device_linux_sysfs.h  |  9 +---
src/libvirt_private.syms   |  5 +
src/node_device/node_device_driver.c   |  8 
src/node_device/node_device_hal.c  |  4 ++--
src/node_device/node_device_udev.c |  4 ++--
7 files changed, 34 insertions(+), 28 deletions(-)
rename src/{node_device => conf}/node_device_linux_sysfs.c (89%)
rename src/{node_device => conf}/node_device_linux_sysfs.h (82%)



What's the reasoning for this move? It somehow doesn't feel right and it
will make backports harder.


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

Re: [libvirt] [RFC PATCH 2/2] lib: Add note that bulk stats API queries may overrun RPC buffers

[Peter Krempa]

On Tue, May 23, 2017 at 10:12:06 +0200, Martin Kletzander wrote:
> On Mon, May 22, 2017 at 06:00:13PM +0200, Peter Krempa wrote:
> > Hint that the users should limit the number of VMs queried in the bulk
> > stats API.
> > ---
> > src/libvirt-domain.c | 7 +++
> > 1 file changed, 7 insertions(+)

[...]

> Is there a discussion about how to continue for the future?  I remember
> Michal starting some discussion, but I'm not sure whether that was about
> the same thing.  Anyway, that's just a rhetorical question so that I can
> say the two ideas I have:
> 
> 1) Just increase the limit over time.  Computers and networks are
>getting faster, there's more storage space and memory, and so on.
>It only makes sense to do scale other things respectively.
> 
> 2) Have a new API that streams the data back over virStream.  We can
>then do it continuously (like every X seconds), that might help
>management apps as well because I suspect that's how they use this
>API anyway.

3) danpb also pointed out that we could use a chunk of shared memory to
transport the stats for local connections, so that we skip the RPC layer
altoghether and also avoid having two copies of the same data.

But that's something we need to design first.


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

Re: [libvirt] [RFC PATCH 2/2] lib: Add note that bulk stats API queries may overrun RPC buffers

[Martin Kletzander]

On Mon, May 22, 2017 at 06:00:13PM +0200, Peter Krempa wrote:

Hint that the users should limit the number of VMs queried in the bulk
stats API.
---
src/libvirt-domain.c | 7 +++
1 file changed, 7 insertions(+)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 310b91b37..b01f2705c 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11315,6 +11315,10 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
 * VIR_CONNECT_GET_ALL_DOMAINS_STATS_SHUTOFF and/or
 * VIR_CONNECT_GET_ALL_DOMAINS_STATS_OTHER for all other states.
 *
+ * Note that this API is prone to exceeding maximum RPC message size on hosts
+ * with lots of VMs so it's suggested to use virDomainListGetStats with a
+ * reasonable list of VMs as the argument.
+ *
 * Returns the count of returned statistics structures on success, -1 on error.
 * The requested data are returned in the @retStats parameter. The returned
 * array should be freed by the caller. See virDomainStatsRecordListFree.
@@ -11386,6 +11390,9 @@ virConnectGetAllDomainStats(virConnectPtr conn,
 * Note that any of the domain list filtering flags in @flags may be rejected
 * by this function.
 *
+ * Note that this API is prone to exceeding maximum RPC message size on hosts
+ * with lots of VMs so it's suggested to limit the number of VMs queried.
+ *


Makes sense to me, now we can at least point people somewhere when this
happens again.  ACK from me, but feel free to wait if you want to really
make this an RFC and you want more opinions.

Is there a discussion about how to continue for the future?  I remember
Michal starting some discussion, but I'm not sure whether that was about
the same thing.  Anyway, that's just a rhetorical question so that I can
say the two ideas I have:

1) Just increase the limit over time.  Computers and networks are
   getting faster, there's more storage space and memory, and so on.
   It only makes sense to do scale other things respectively.

2) Have a new API that streams the data back over virStream.  We can
   then do it continuously (like every X seconds), that might help
   management apps as well because I suspect that's how they use this
   API anyway.

Thanks for listening ;)


 * Returns the count of returned statistics structures on success, -1 on error.
 * The requested data are returned in the @retStats parameter. The returned
 * array should be freed by the caller. See virDomainStatsRecordListFree.
--
2.12.2

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


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

Re: [libvirt] [RFC PATCH 1/2] rpc: Bump maximum message size to 32M

[Martin Kletzander]

On Mon, May 22, 2017 at 06:00:12PM +0200, Peter Krempa wrote:

While most of the APIs are okay with 16M messages, the bulk stats API
can run into the limit in big configurations. Before we devise a new
plan for this, bump this limit slightly to accomodate some more configs.
---
src/rpc/virnetprotocol.x | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/rpc/virnetprotocol.x b/src/rpc/virnetprotocol.x
index ee9899059..901c67159 100644
--- a/src/rpc/virnetprotocol.x
+++ b/src/rpc/virnetprotocol.x
@@ -40,13 +40,13 @@ const VIR_NET_MESSAGE_INITIAL = 65536;
const VIR_NET_MESSAGE_LEGACY_PAYLOAD_MAX = 262120;

/* Maximum total message size (serialised). */
-const VIR_NET_MESSAGE_MAX = 16777216;
+const VIR_NET_MESSAGE_MAX = 33554432;



ACK.

Although if you changed it to something nicer to read so that it doesn't
take someone too long to figure out it's actually just 32 MiB, that
would be even better.  Whatever suits you, be it (1 << 25) or (32 * 2 <<
20) or (32 * 1024 * 1024) or anything else.  I know other numbers could
be changed as well, but this one is most updated, currently.


/* Size of struct virNetMessageHeader (serialised)*/
const VIR_NET_MESSAGE_HEADER_MAX = 24;

/* Size of message payload */
-const VIR_NET_MESSAGE_PAYLOAD_MAX = 16777192;
+const VIR_NET_MESSAGE_PAYLOAD_MAX = 33554408;



and this could be:

const VIR_NET_MESSAGE_PAYLOAD_MAX = VIR_NET_MESSAGE_MAX - 
VIR_NET_MESSAGE_HEADER_MAX;

or is none of that supported by the parser?  In that case, just leave it
like that.


/* Size of message length field. Not counted in VIR_NET_MESSAGE_MAX
 * and VIR_NET_MESSAGE_INITIAL.
--
2.12.2

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


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

Re: [libvirt] libvirt-question

[zhun...@gmail.com]

thanks a lot!



zhun...@gmail.com
 
From: Erik Skultety
Date: 2017-05-23 15:09
To: zhun...@gmail.com
CC: libvir-list
Subject: Re: [libvirt] libvirt-question
On Tue, May 23, 2017 at 10:29:14AM +0800, zhun...@gmail.com wrote:
> Hello,does libvirt provides API to get qemu process ID by vm ID or name??or 
> is there any methods to do this??
> thanks!!
 
No, libvirt doesn't provide such API. Depends on what you really want to do.
Libvirt uses domain objects to identify a domain and that's all you need to
operate on domains. If you want to operate on the process level, for whatever
administration reasons, that's out of libvirt's scope.
 
Erik
 
>
>
>
> zhun...@gmail.com
 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
 
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] libvirt-question

[Martin Kletzander]

On Tue, May 23, 2017 at 10:29:14AM +0800, zhun...@gmail.com wrote:

Hello,does libvirt provides API to get qemu process ID by vm ID or name??or is 
there any methods to do this??
thanks!!



No.  It was discussed multiple times here.  I added explanation [1] to
our FAQ.

[1] 
http://wiki.libvirt.org/page/FAQ#How_can_I_get_QEMU.27s_PID_for_a_VM_using_libvirt_APIs.3F




zhun...@gmail.com



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


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

Re: [libvirt] libvirt-question

[Erik Skultety]

On Tue, May 23, 2017 at 10:29:14AM +0800, zhun...@gmail.com wrote:
> Hello,does libvirt provides API to get qemu process ID by vm ID or name??or 
> is there any methods to do this??
> thanks!!

Oh, and I also forgot to mention that these kinds of questions are better suited
for libvirt-users mailing list. libvir-list is where the libvirt's upstream
work (ideas/discussions/development) takes place.

Regards,
Erik

>
>
>
> zhun...@gmail.com

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

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

Re: [libvirt] [PATCH V2] libxl: add default controllers for USB devices

[Cedric Bosdonnat]

On Fri, 2017-05-05 at 15:48 -0600, Jim Fehlig wrote:
> Attempting to start a domain with USB hostdevs but no USB controllers
> fails with the rather cryptic error
> 
> libxl: error: libxl_qmp.c:287:qmp_handle_error_response: received an
> error message from QMP server: Bus 'xenusb-0.0' not found
> 
> This can be fixed by creating default USB controllers. When no USB
> controllers are defined, create the number of 8 port controllers
> necessary to accommodate the number of defined USB devices.
> 
> Note that USB controllers are already created as needed in the
> domainAttachDevice code path. E.g. a USB controller will be created,
> if necessary, when attaching a USB device with
> 'virsh attach-device dom usbdev.xml'.
> 
> Signed-off-by: Jim Fehlig 
> ---
> 
> V1 here
> 
> https://www.redhat.com/archives/libvir-list/2017-April/msg00965.html
> 
> While further testing of V1 found a libvirtd segfault due to
> incorrectly using virDomainControllerInsertPreAlloced instead of
> virDomainControllerInsert.
> 
>  src/libxl/libxl_conf.c | 82 
> +++---
>  1 file changed, 71 insertions(+), 11 deletions(-)
> 
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 56bc09719..cdf6ec9f3 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -1822,34 +1822,94 @@ libxlMakeUSBController(virDomainControllerDefPtr 
> controller,
>  }
>  
>  static int
> +libxlMakeDefaultUSBControllers(virDomainDefPtr def,
> +   libxl_domain_config *d_config)
> +{
> +virDomainControllerDefPtr l_controller = NULL;
> +libxl_device_usbctrl *x_controllers = NULL;
> +size_t nusbdevs = 0;
> +size_t ncontrollers;
> +size_t i;
> +
> +for (i = 0; i < def->nhostdevs; i++) {
> +if (def->hostdevs[i]->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
> +def->hostdevs[i]->source.subsys.type == 
> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
> +nusbdevs++;
> +}
> +
> +/* No controllers needed if there are no USB devs */
> +if (nusbdevs == 0)
> +return 0;
> +
> +/* Create USB controllers with 8 ports */
> +ncontrollers = VIR_DIV_UP(nusbdevs, 8);
> +if (VIR_ALLOC_N(x_controllers, ncontrollers) < 0)
> +return -1;
> +
> +for (i = 0; i < ncontrollers; i++) {
> +if (!(l_controller = 
> virDomainControllerDefNew(VIR_DOMAIN_CONTROLLER_TYPE_USB)))
> +goto error;
> +
> +l_controller->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB2;
> +l_controller->idx = i;
> +l_controller->opts.usbopts.ports = 8;
> +
> +libxl_device_usbctrl_init(_controllers[i]);
> +
> +if (libxlMakeUSBController(l_controller, _controllers[i]) < 0)
> +goto error;
> +
> +if (virDomainControllerInsert(def, l_controller) < 0)
> +goto error;
> +
> +l_controller = NULL;
> +}
> +
> +d_config->usbctrls = x_controllers;
> +d_config->num_usbctrls = ncontrollers;
> +return 0;
> +
> + error:
> + virDomainControllerDefFree(l_controller);
> + for (i = 0; i < ncontrollers; i++)
> + libxl_device_usbctrl_dispose(_controllers[i]);
> + VIR_FREE(x_controllers);
> + return -1;
> +}
> +
> +static int
>  libxlMakeUSBControllerList(virDomainDefPtr def, libxl_domain_config 
> *d_config)
>  {
>  virDomainControllerDefPtr *l_controllers = def->controllers;
>  size_t ncontrollers = def->ncontrollers;
>  size_t nusbctrls = 0;
>  libxl_device_usbctrl *x_usbctrls;
> -size_t i;
> -
> -if (ncontrollers == 0)
> -return 0;
> -
> -if (VIR_ALLOC_N(x_usbctrls, ncontrollers) < 0)
> -return -1;
> +size_t i, j;
>  
>  for (i = 0; i < ncontrollers; i++) {
> +if (l_controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_USB)
> +nusbctrls++;
> +}
> +
> +if (nusbctrls == 0)
> +return libxlMakeDefaultUSBControllers(def, d_config);
> +
> +if (VIR_ALLOC_N(x_usbctrls, nusbctrls) < 0)
> +return -1;
> +
> +for (i = 0, j = 0; i < ncontrollers && j < nusbctrls; i++) {
>  if (l_controllers[i]->type != VIR_DOMAIN_CONTROLLER_TYPE_USB)
>  continue;
>  
> -libxl_device_usbctrl_init(_usbctrls[nusbctrls]);
> +libxl_device_usbctrl_init(_usbctrls[j]);
>  
>  if (libxlMakeUSBController(l_controllers[i],
> -   _usbctrls[nusbctrls]) < 0)
> +   _usbctrls[j]) < 0)
>  goto error;
>  
> -nusbctrls++;
> +j++;
>  }
>  
> -VIR_SHRINK_N(x_usbctrls, ncontrollers, ncontrollers - nusbctrls);
>  d_config->usbctrls = x_usbctrls;
>  d_config->num_usbctrls = nusbctrls;
>  

ACK

--
Cedric

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

Re: [libvirt] libvirt-question

[Erik Skultety]

On Tue, May 23, 2017 at 10:29:14AM +0800, zhun...@gmail.com wrote:
> Hello,does libvirt provides API to get qemu process ID by vm ID or name??or 
> is there any methods to do this??
> thanks!!

No, libvirt doesn't provide such API. Depends on what you really want to do.
Libvirt uses domain objects to identify a domain and that's all you need to
operate on domains. If you want to operate on the process level, for whatever
administration reasons, that's out of libvirt's scope.

Erik

>
>
>
> zhun...@gmail.com

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

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