[libvirt] [PATCH 1/2] xenapi: remove driver

[Jim Fehlig]

The xenapi driver has not seen any development since its initial
contribution 9 years ago. There have been no bug reports, no patches,
and no queries about the driver on the developer or user mailing lists.
Remove the driver from the libvirt sources.

Signed-off-by: Jim Fehlig 
---
 cfg.mk |4 +-
 configure.ac   |4 -
 docs/aclpolkit.html.in |4 -
 docs/auth.html.in  |2 -
 libvirt.spec.in|4 +-
 m4/virt-driver-xenapi.m4   |   48 -
 po/POTFILES|2 -
 src/Makefile.am|1 -
 src/README |1 -
 src/libvirt.c  |   10 -
 src/util/virrandom.c   |3 +-
 src/xenapi/Makefile.inc.am |   29 -
 src/xenapi/xenapi_driver.c | 2125 
 src/xenapi/xenapi_driver.h |   22 -
 src/xenapi/xenapi_driver_private.h |   58 -
 src/xenapi/xenapi_utils.c  |  562 
 src/xenapi/xenapi_utils.h  |   76 -
 tools/virsh.c  |3 -
 18 files changed, 3 insertions(+), 2955 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index 1f29729949..42e1abf0dd 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -611,7 +611,6 @@ msg_gen_function += virRaiseError
 msg_gen_function += virReportError
 msg_gen_function += virReportErrorHelper
 msg_gen_function += virReportSystemError
-msg_gen_function += xenapiSessionErrorHandler
 msg_gen_function += virLastErrorPrefixMessage
 
 # Uncomment the following and run "make syntax-check" to see diagnostics
@@ -668,7 +667,7 @@ sc_prohibit_diagnostic_without_format:
   $(VC_LIST_EXCEPT) | xargs \
$(GREP) -A2 -nE '\<$(func_re) *\(.*,$$' /dev/null; } \
   | $(SED) -rn -e ':l; /[,"]$$/ {N;b l;}' \
-   -e '/(xenapiSessionErrorHandler|vah_(error|warning))/d' \
+   -e '/(vah_(error|warning))/d' \
-e '/\<$(func_re) *\([^"]*"([^%"]|"\n[^"]*")*"[,)]/p' \
| $(GREP) -vE 'VIR_ERROR' && \
  { echo '$(ME): found diagnostic without %' 1>&2; \
@@ -791,7 +790,6 @@ sc_prohibit_cross_inclusion:
access/ | conf/) safe="($$dir|conf|util)";; \
cpu/| network/| node_device/| rpc/| security/| storage/) \
  safe="($$dir|util|conf|storage)";; \
-   xenapi/) safe="($$dir|util|conf|xen|cpu)";; \
*) safe="($$dir|$(mid_dirs)|util)";; \
  esac; \
  in_vc_files="^src/$$dir" \
diff --git a/configure.ac b/configure.ac
index 7c76a9c9ec..19d8937a8a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -456,7 +456,6 @@ LIBVIRT_DRIVER_ARG_QEMU
 LIBVIRT_DRIVER_ARG_OPENVZ
 LIBVIRT_DRIVER_ARG_VMWARE
 LIBVIRT_DRIVER_ARG_PHYP
-LIBVIRT_DRIVER_ARG_XENAPI
 LIBVIRT_DRIVER_ARG_LIBXL
 LIBVIRT_DRIVER_ARG_VBOX
 LIBVIRT_DRIVER_ARG_LXC
@@ -474,7 +473,6 @@ LIBVIRT_DRIVER_CHECK_QEMU
 LIBVIRT_DRIVER_CHECK_OPENVZ
 LIBVIRT_DRIVER_CHECK_VMWARE
 LIBVIRT_DRIVER_CHECK_PHYP
-LIBVIRT_DRIVER_CHECK_XENAPI
 LIBVIRT_DRIVER_CHECK_LIBXL
 LIBVIRT_DRIVER_CHECK_VBOX
 LIBVIRT_DRIVER_CHECK_LXC
@@ -960,7 +958,6 @@ LIBVIRT_DRIVER_RESULT_QEMU
 LIBVIRT_DRIVER_RESULT_OPENVZ
 LIBVIRT_DRIVER_RESULT_VMWARE
 LIBVIRT_DRIVER_RESULT_VBOX
-LIBVIRT_DRIVER_RESULT_XENAPI
 LIBVIRT_DRIVER_RESULT_LIBXL
 LIBVIRT_DRIVER_RESULT_LXC
 LIBVIRT_DRIVER_RESULT_PHYP
@@ -1041,7 +1038,6 @@ LIBVIRT_RESULT_SSH2
 LIBVIRT_RESULT_UDEV
 LIBVIRT_RESULT_VIRTUALPORT
 LIBVIRT_RESULT_XDR
-LIBVIRT_RESULT_XENAPI
 LIBVIRT_RESULT_YAJL
 AC_MSG_NOTICE([])
 AC_MSG_NOTICE([Windows])
diff --git a/docs/aclpolkit.html.in b/docs/aclpolkit.html.in
index 2cf1f9b5a5..68e6d399b2 100644
--- a/docs/aclpolkit.html.in
+++ b/docs/aclpolkit.html.in
@@ -393,10 +393,6 @@
   vz
   vz
 
-
-  xenapi
-  XenAPI
-
   
 
 
diff --git a/docs/auth.html.in b/docs/auth.html.in
index 33afe0a8ad..62af43a915 100644
--- a/docs/auth.html.in
+++ b/docs/auth.html.in
@@ -132,8 +132,6 @@ credentials=defgrp
 over SSH
   esx - used for connections to an ESX or
 VirtualCenter server
-  xen - used for connections to a Xen Enterprise
-sever using XenAPI
 
 
 
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 41c4a142d6..dde8bf3a97 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -118,14 +118,13 @@
 %endif
 
 # RHEL doesn't ship OpenVZ, VBox, PowerHypervisor,
-# VMware, libxenserver (xenapi), libxenlight (Xen 4.1 and newer),
+# VMware, libxenlight (Xen 4.1 and newer),
 # or HyperV.
 %if 0%{?rhel}
 %define with_openvz 0
 %define with_vbox 0
 %define with_phyp 0
 %define with_vmware 0
-%define with_xenapi 0
 %define with_libxl 0
 %define with_hyperv 0
 %define with_vz 0
@@ -1169,7 +1168,6 @@ rm -f po/stamp-po
%{?arg_esx} \
%{?arg_hyperv} \
%{?arg_vmware} \
-   --without-xenapi \
--without-vz \
--without-bhyve \

[libvirt] [PATCH 2/2] news: Mention removal of xenapi driver

[Jim Fehlig]

Signed-off-by: Jim Fehlig 
---
 docs/news.xml | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index 92d566c2fe..303078036d 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -63,6 +63,16 @@
   KVM device assignment from libvirt too.
 
   
+  
+
+  Remove xenapi driver
+
+
+  The xenapi driver is removed since it has not received any 
significant
+  development since its initial contribution nine years ago and has no
+  known user base.
+
+  
 
 
   
-- 
2.22.0


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

[libvirt] [PATCH 0/2] Remove xenapi driver

[Jim Fehlig]

Remove the xenapi driver is unmaintained and has no known user base.
We are stuck with references in include/libvirt/virterror.h and
src/util/virerror.c but otherwise I think I've nuked xenapi everywhere.

Jim Fehlig (2):
  xenapi: remove driver
  news: Mention removal of xenapi driver

 cfg.mk |4 +-
 configure.ac   |4 -
 docs/aclpolkit.html.in |4 -
 docs/auth.html.in  |2 -
 docs/news.xml  |   10 +
 libvirt.spec.in|4 +-
 m4/virt-driver-xenapi.m4   |   48 -
 po/POTFILES|2 -
 src/Makefile.am|1 -
 src/README |1 -
 src/libvirt.c  |   10 -
 src/util/virrandom.c   |3 +-
 src/xenapi/Makefile.inc.am |   29 -
 src/xenapi/xenapi_driver.c | 2125 
 src/xenapi/xenapi_driver.h |   22 -
 src/xenapi/xenapi_driver_private.h |   58 -
 src/xenapi/xenapi_utils.c  |  562 
 src/xenapi/xenapi_utils.h  |   76 -
 tools/virsh.c  |3 -
 19 files changed, 13 insertions(+), 2955 deletions(-)
 delete mode 100644 m4/virt-driver-xenapi.m4
 delete mode 100644 src/xenapi/Makefile.inc.am
 delete mode 100644 src/xenapi/xenapi_driver.c
 delete mode 100644 src/xenapi/xenapi_driver.h
 delete mode 100644 src/xenapi/xenapi_driver_private.h
 delete mode 100644 src/xenapi/xenapi_utils.c
 delete mode 100644 src/xenapi/xenapi_utils.h

-- 
2.22.0


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

Re: [libvirt] [PATCH for 5.7.0 0/2] virsh: Rename --precopy-bandwidth migration option

[Jim Fehlig]

On 9/2/19 9:08 AM, Jiri Denemark wrote:
> The (pre-copy) bandwidth was historically the only bandwidth we
> supported and thus it is called just "bandwidth" in all other places.
> E.g., virsh migrate-setspeed or in the migration typed parameter name.
> Let's make the new option for virsh migrate consistent.
> 
> Jiri Denemark (2):
>virsh: Rename --precopy-bandwidth migration option
>news: Rename --precopy-bandwidth migration option
> 
>   docs/news.xml| 4 ++--
>   tools/virsh-domain.c | 6 +++---
>   tools/virsh.pod  | 4 ++--
>   3 files changed, 7 insertions(+), 7 deletions(-)
> 

I was a little uncertain about the name and mentioned that when submitting the 
patch. Agreed that "bandwidth" is more consistent and a better choice.

Reviewed-by: Jim Fehlig 

Regards,
Jim

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

Re: [libvirt] Error when creating VM with persistent memory

[Seema Pandit]

After adding the memoryBacking tag in xml as below (in addition, to other
xml changes to add nvdimm), virsh could allocate AD memory larger than the
system RAM and VMs could start successfully.



   



  



This adds share=yes in command line.



-object
memory-backend-file,id=memnvdimm0,prealloc=yes,mem-path=/mnt/pmem0/file1,share=yes,size=414464344064
-device nvdimm,node=0,label-size=131072,memdev=memnvdimm0,id=nvdimm0,slot=0





For reference qemu command line where VM starts quickly:

qemu-system-x86_64 \

-name qemu-gold29 \

-drive
file=/var/lib/libvirt/images/gold29-ad.qcow2,format=qcow2,index=0,media=disk
\ -m 2G,slots=4,maxmem=428G \ -smp 2 \ -machine pc,accel=kvm,nvdimm=on \
-enable-kvm \ -object
memory-backend-file,id=pmem1,share=on,mem-path=/mnt/pmem0/file1,size=386G,align=4K
\ -device nvdimm,memdev=pmem1,id=nv1 \ -daemonize





Qemu command line generated from virsh: (please note VM now starts with
this command line, shared=yes.)



 /usr/bin/qemu-system-x86_64 -machine accel=kvm -name
guest=mix-test,debug-threads=on -S -object
secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-18-mix-test/master-key.aes
-machine
pc-i440fx-3.0,accel=kvm,usb=off,vmport=off,dump-guest-core=off,nvdimm=on
-cpu Skylake-Server-IBRS,hypervisor=on -m
size=2097152k,slots=16,maxmem=419430400k -realtime mlock=off -smp
2,sockets=2,cores=1,threads=1 -object
memory-backend-file,id=ram-node0,mem-path=/var/lib/libvirt/qem
/ram/libvirt/qemu/18-mix-test/ram-node0,discard-data=yes,share=yes,size=2147483648
-numa node,nodeid=0,cpus=0-1,memdev=ram-node0 -object
memory-backend-file,id=memnvdimm0,prealloc=yes,mem-path=/mnt/pmem0/file1,share=yes,size=414464344064
-device nvdimm,node=0,label-size=131072,memdev=memnvdimm0,id=nvdimm0,slot=0
-uuid 318c0529-0330-460b-8d0a-3b253e9decdd -no-user-config -nodefaults
-chardev socket,id=charmonitor,fd=32,server,nowait -mon
chardev=charmonitor,id=monitor,mode=control -rtc base=utc,driftfix=slew
-global kvm-pit.lost_tick_policy=delay -no-hpet -no-shutdown -global
PIIX4_PM.disable_s3=1 -global PIIX4_PM.disable_s4=1 -boot strict=on -device
ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x5.0x7 -device
ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x5
-device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x5.0x1
-device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x5.0x2
-device virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x6 -drive
file=/var/lib/libvirt/images/mix-test.qcow2,format=qcow2,if=none,id=drive-virtio-disk0
-device
virtio-blk-pci,scsi=off,bus=pci.0,addr=0x7,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1
-drive if=none,id=drive-ide0-0-0,readonly=on -device
ide-cd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 -netdev
tap,fd=34,id=hostnet0 -device
e1000,netdev=hostnet0,id=net0,mac=52:54:00:06:db:55,bus=pci.0,addr=0xa
-chardev pty,id=charserial0 -device
isa-serial,chardev=charserial0,id=serial0 -chardev
socket,id=charchannel0,fd=35,server,nowait -device
virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=org.qemu.guest_agent.0
-chardev spicevmc,id=charchannel1,name=vdagent -device
virtserialport,bus=virtio-serial0.0,nr=2,chardev=charchannel1,id=channel1,name=com.redhat.spice.0
-device usb-tablet,id=input0,bus=usb.0,port=1 -spice
port=5901,addr=127.0.0.1,disable-ticketing,image-compression=off,seamless-migration=on
-device
qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,vram64_size_mb=0,vgamem_mb=16,max_outputs=1,bus=pci.0,addr=0x2
-device intel-hda,id=sound0,bus=pci.0,addr=0x4 -device
hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 -chardev
spicevmc,id=charredir0,name=usbredir -device
usb-redir,chardev=charredir0,id=redir0,bus=usb.0,port=2 -chardev
spicevmc,id=charredir1,name=usbredir -device
usb-redir,chardev=charredir1,id=redir1,bus=usb.0,port=3 -device
virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x8 -object
rng-random,id=objrng0,filename=/dev/urandom -device
virtio-rng-pci,rng=objrng0,id=rng0,bus=pci.0,addr=0x9 -sandbox
on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny
-msg timestamp=on


VM start takes longer that qemu. Any thoughts?? Why is prealloc=yes default
on for nvdimm? Any other important deltas?



On Mon, Sep 2, 2019 at 12:21 PM Michal Privoznik 
wrote:

> On 8/31/19 7:33 PM, Seema Pandit wrote:
> > Hi Michal,
> > Thank you for the reply.
> > I was having issues compiling qemu code on fedora29. So instead of
> dropping
> > prealloc in virsh, tried adding prealloc=yes in qemu command line.
> > prealloc=yes works. It does not lead to using more system memory when
> using
> > DAX.
> > +Dan
> > Here are the steps:
> >
> > ndctl create-namespace -t pmem -m fsdax --align=4k -s 400G
> >
> > mkfs.ext4 /dev/pmem0
> >
> > mount -o dax /dev/pmem0 /mnt/pmem0
> >
> > dd if=/dev/zero of=/mnt/pmem0/file1 bs=4k count=104857600
> >
> > [root@system-name]# dd if=/dev/zero of=/mnt/pmem0/file1 bs=4k
> > count=104857600
> >
> > dd: error writing 

[libvirt] [PATCH v2] util: Set backing file name for LOOP_GET_STATUS64 queries.

[jcfaracco]

From: Julio Faracco 

This is an issue for LXC loop devices when you are trying to get loop
devices info using `ioctl`. Modern apps uses `/sys/dev/block` to grab
information about devices, but if you use the method mention you won't
be able to retrive the associated file with that loop device. See
example below from cryptsetup sources:

static char *_ioctl_backing_file(const char *loop)
{
struct loop_info64 lo64 = {0};
int loop_fd;

loop_fd = open(loop, O_RDONLY);
if (loop_fd < 0)
return NULL;

if (ioctl(loop_fd, LOOP_GET_STATUS64, ) < 0) {
close(loop_fd);
return NULL;
}

lo64.lo_file_name[LO_NAME_SIZE-2] = '*';
lo64.lo_file_name[LO_NAME_SIZE-1] = 0;

close(loop_fd);
return strdup((char*)lo64.lo_file_name);
}

It will return an empty string because lo_file_name was not set.
Function `virFileLoopDeviceOpenSearch()` is using `ioctl` to query data,
but it is not checking `lo_file_name` field.

v1: I accidentally committed two wrong lines.

Signed-off-by: Julio Faracco 
---
 src/util/virfile.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 81a3c096eb..dbfe74e24f 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -781,6 +781,14 @@ int virFileLoopDeviceAssociate(const char *file,
 memset(, 0, sizeof(lo));
 lo.lo_flags = LO_FLAGS_AUTOCLEAR;
 
+/* Set backing file name for LOOP_GET_STATUS64 queries */
+if (virStrncpy((char *) lo.lo_file_name, file,
+   strlen(file), LO_NAME_SIZE) < 0) {
+virReportSystemError(errno,
+ _("Unable to set backing file %s"), file);
+goto cleanup;
+}
+
 if ((fsfd = open(file, O_RDWR)) < 0) {
 virReportSystemError(errno,
  _("Unable to open %s"), file);
-- 
2.20.1

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

[libvirt] [PATCH 0/1] Rewrite virt-host-validate in Go & Go packaging complication

[Daniel P . Berrangé]

This series contains code I mostly wrote some 6 months ago
as an experiment. I've tidied it up and attempted to integrate
it into the libvirt build system.

Overall I much prefer the new impl of virt-host-validate, as
explained in the commit message, I've wanted todo it this way
for a long time, but was put off by C's XML handling verbosity.

The main problem I've hit in integrating this work is the RPM
build process integration.

When building interactively, Go will dynamically download the
3rd party deps from git, similar to how we dynamically pull
down gnulib. The difference is that this must happen for end
users using tarballs too, not just devs usingn git.

We could bundle the deps with our tarball the same way we do
with gnulib, but Fedora would not accept using this, as it
wants unbundled Go deps.

I've done some things in the spec which makes this work
against unbundled sources, but this still fails in mock as
we don't get the build-id set in the resulting binary. I
have attempted to fix that by passing extra flags to go
build, but it still fails, so I've got some bug there. The
resulting spec rules are very very very unpleasant, having
to hardcode a bunch of build flags we should never touch.

I've spoken with Nichlos Mailhot who is the Fedora Go
packaging expert, who has basically confirmed my experiance
that there is no nice solution to what I'm trying todo with
the build process. He strongly recommends that we do *not*
try to integrate Go into our existing build process, and that
we instead keep any Go code as a completely seperate git repo
that we build & install separately. This would make the
RPM packaging much much simpler to deal with.

Given what I've tried here, I'm inclined to agree with his
recommendation.

In the case of virt-host-validate, this is actually quite
a straightforward thing to deal with, as the new impl of
virt-host-validate has no direct dep wrt existing lkibvirt
code in either direction. So we can trivially put it in a
separate git repo. We would merely need an RPM dep from
libvirt.spec.in to make the sure the external  project
gets pulled in on install of libvirt-daemon.

Thus this patch is *NOT* something I'm proposing to merge
as is. The code itself is fine, but it will need to go into
a new git repo of its own.

Daniel P. Berrangé (1):
  tools: rewrite virt-host-validate in Go to be data driven

 configure.ac |   1 +
 libvirt.spec.in  |  23 +
 m4/virt-golang.m4|  46 ++
 m4/virt-host-validate.m4 |   8 +-
 po/POTFILES  |   5 -
 tools/Makefile.am|  59 +-
 tools/host-validate/go.mod   |   8 +
 tools/host-validate/go.sum   |   4 +
 tools/host-validate/main.go  |  98 +++
 tools/host-validate/pkg/engine.go| 469 +++
 tools/host-validate/pkg/facts.go | 784 +++
 tools/host-validate/pkg/facts_test.go|  67 ++
 tools/host-validate/pkg/xml_utils.go | 287 +++
 tools/host-validate/rules/acpi.xml   |  29 +
 tools/host-validate/rules/builtin.xml|  33 +
 tools/host-validate/rules/cgroups.xml| 403 ++
 tools/host-validate/rules/cpu.xml| 148 
 tools/host-validate/rules/devices.xml|  54 ++
 tools/host-validate/rules/freebsd-kernel.xml |  14 +
 tools/host-validate/rules/iommu.xml  |  93 +++
 tools/host-validate/rules/namespaces.xml |  94 +++
 tools/host-validate/rules/pci.xml|   9 +
 tools/virt-host-validate-bhyve.c |  77 --
 tools/virt-host-validate-bhyve.h |  24 -
 tools/virt-host-validate-common.c| 419 --
 tools/virt-host-validate-common.h|  85 --
 tools/virt-host-validate-lxc.c   |  87 --
 tools/virt-host-validate-lxc.h   |  24 -
 tools/virt-host-validate-qemu.c  | 116 ---
 tools/virt-host-validate-qemu.h  |  24 -
 tools/virt-host-validate.c   | 152 
 tools/virt-host-validate.pod |  12 +-
 32 files changed, 2693 insertions(+), 1063 deletions(-)
 create mode 100644 m4/virt-golang.m4
 create mode 100644 tools/host-validate/go.mod
 create mode 100644 tools/host-validate/go.sum
 create mode 100644 tools/host-validate/main.go
 create mode 100644 tools/host-validate/pkg/engine.go
 create mode 100644 tools/host-validate/pkg/facts.go
 create mode 100644 tools/host-validate/pkg/facts_test.go
 create mode 100644 tools/host-validate/pkg/xml_utils.go
 create mode 100644 tools/host-validate/rules/acpi.xml
 create mode 100644 tools/host-validate/rules/builtin.xml
 create mode 100644 tools/host-validate/rules/cgroups.xml
 create mode 100644 tools/host-validate/rules/cpu.xml
 create mode 100644 tools/host-validate/rules/devices.xml
 create mode 100644 tools/host-validate/rules/freebsd-kernel.xml
 create 

Re: [libvirt] [PATCH for 5.7.0 0/2] virsh: Rename --precopy-bandwidth migration option

[Daniel P . Berrangé]

On Mon, Sep 02, 2019 at 05:08:12PM +0200, Jiri Denemark wrote:
> The (pre-copy) bandwidth was historically the only bandwidth we
> supported and thus it is called just "bandwidth" in all other places.
> E.g., virsh migrate-setspeed or in the migration typed parameter name.
> Let's make the new option for virsh migrate consistent.
> 
> Jiri Denemark (2):
>   virsh: Rename --precopy-bandwidth migration option
>   news: Rename --precopy-bandwidth migration option
> 
>  docs/news.xml| 4 ++--
>  tools/virsh-domain.c | 6 +++---
>  tools/virsh.pod  | 4 ++--
>  3 files changed, 7 insertions(+), 7 deletions(-)

I'm fine with this going in for 5.7.0

Reviewed-by: Daniel P. Berrangé 

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

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

[libvirt] [PATCH python] spec: Build python2 package in fedora < 31

[Nir Soffer]

Since commit ee0cfbe65c5d (spec: Unconditionally build python2 on
Fedora) python2-libvirt is not built on any Fedora version.

Fix the spec to drop python2-libvirt on Fedora 31.

Signed-off-by: Nir Soffer 
---
 libvirt-python.spec.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libvirt-python.spec.in b/libvirt-python.spec.in
index 64a30b5..8506840 100644
--- a/libvirt-python.spec.in
+++ b/libvirt-python.spec.in
@@ -13,7 +13,7 @@
 %endif
 
 %define _with_python2 1
-%if 0%{?fedora} || 0%{?rhel} > 7
+%if 0%{?fedora} > 30 || 0%{?rhel} > 7
 %define _with_python2 0
 %endif
 
-- 
2.20.1

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

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

[Michal Privoznik]

On 9/2/19 4:16 PM, Daniel P. Berrangé wrote:

On Mon, Sep 02, 2019 at 02:07:08PM +, Jim Fehlig wrote:

On 8/31/19 2:11 AM, Michal Prívozník  wrote:

On 8/26/19 1:49 PM, Ján Tomko wrote:

The 'From:' field shows your e-mail in uppercase.

On Fri, Aug 23, 2019 at 07:50:12PM +, Jim Fehlig wrote:

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





Actually, this breaks --with-xenapi build:

xenapi/xenapi_driver.c:38:10: fatal error: xen_common.h: No such file or
directory
   #include "xen_common.h"
^~
compilation terminated.


Can you determine why this is needed? E.g. remove it and see what subsequently
fails? I don't have a setup readily available to test a xenapi build (I'm not
"in the office" today as it is a US holiday).


I'm inclined to say that we should call the XenAPI driver dead at
this point. IIUC, the person at Citrix who wrote left, left that
job shortly after it was merged into libvirt. Looking at the git
history I'm struggling to see any single patch applied in 9 years
since that was not a general cleanup/bugfix. The XML handling does
not deal with disks at all AFAICT making it largely useless.

IOW, lets just document that it fails to build for this release
and then delete its code.


Agreed.

Michal

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

[libvirt] [PATCH 1/2] virsh: Rename --precopy-bandwidth migration option

[Jiri Denemark]

The (pre-copy) bandwidth was historically the only bandwidth we
supported and thus it is called just "bandwidth" in all other places.
E.g., virsh migrate-setspeed or in the migration typed parameter name.
Let's make the new option for virsh migrate consistent.

Signed-off-by: Jiri Denemark 
---
 tools/virsh-domain.c | 6 +++---
 tools/virsh.pod  | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 201fc176c7..3d26e81b22 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -10587,9 +10587,9 @@ static const vshCmdOptDef opts_migrate[] = {
  .type = VSH_OT_INT,
  .help = N_("number of connections for parallel migration")
 },
-{.name = "precopy-bandwidth",
+{.name = "bandwidth",
  .type = VSH_OT_INT,
- .help = N_("pre-copy migration bandwidth limit in MiB/s")
+ .help = N_("migration bandwidth limit in MiB/s")
 },
 {.name = NULL}
 };
@@ -10805,7 +10805,7 @@ doMigrate(void *opaque)
 goto save_error;
 }
 
-if ((rv = vshCommandOptULongLong(ctl, cmd, "precopy-bandwidth", )) 
< 0) {
+if ((rv = vshCommandOptULongLong(ctl, cmd, "bandwidth", )) < 0) {
 goto out;
 } else if (rv > 0) {
 if (virTypedParamsAddULLong(, , ,
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 0dc3216bc2..59fb4bfc2e 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2153,7 +2153,7 @@ I I [I] [I] 
[I] [I] [I<--persistent-xml> B] [I<--tls>]
 [I<--postcopy-bandwidth> B]
 [I<--parallel> [I<--parallel-connections> B]]
-[I<--precopy-bandwidth> B]
+[I<--bandwidth> B]
 
 Migrate domain to another host.  Add I<--live> for live migration; <--p2p>
 for peer-2-peer migration; I<--direct> for direct migration; or I<--tunnelled>
@@ -2186,7 +2186,7 @@ I<--postcopy-after-precopy> along with I<--postcopy> to 
let libvirt
 automatically switch to post-copy after the first pass of pre-copy is finished.
 The maximum bandwidth consumed during the post-copy phase may be limited using
 I<--postcopy-bandwidth>. The maximum bandwidth consumed during the pre-copy 
phase
-may be limited using I<--precopy-bandwidth>.
+may be limited using I<--bandwidth>.
 
 I<--auto-converge> forces convergence during live migration. The initial
 guest CPU throttling rate can be set with I. If the
-- 
2.23.0

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

[libvirt] [PATCH 2/2] news: Rename --precopy-bandwidth migration option

[Jiri Denemark]

The (pre-copy) bandwidth was historically the only bandwidth we
supported and thus it is called just "bandwidth" in all other places.
E.g., virsh migrate-setspeed or in the migration typed parameter name.
Let's make the new option for virsh migrate consistent.

Signed-off-by: Jiri Denemark 
---
 docs/news.xml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/news.xml b/docs/news.xml
index 92d566c2fe..0c79765fd0 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -67,12 +67,12 @@
 
   
 
-  virsh: Support setting precopy bandwidth in migrate subcommand
+  virsh: Support setting bandwidth in migrate subcommand
 
 
   In addition to postcopy bandwidth, the virsh migrate
   subcommand now supports specifying precopy bandwidth with the
-  --precopy-bandwidth parameter.
+  --bandwidth parameter.
 
   
 
-- 
2.23.0

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

[libvirt] [PATCH for 5.7.0 0/2] virsh: Rename --precopy-bandwidth migration option

[Jiri Denemark]

The (pre-copy) bandwidth was historically the only bandwidth we
supported and thus it is called just "bandwidth" in all other places.
E.g., virsh migrate-setspeed or in the migration typed parameter name.
Let's make the new option for virsh migrate consistent.

Jiri Denemark (2):
  virsh: Rename --precopy-bandwidth migration option
  news: Rename --precopy-bandwidth migration option

 docs/news.xml| 4 ++--
 tools/virsh-domain.c | 6 +++---
 tools/virsh.pod  | 4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

-- 
2.23.0

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

Re: [libvirt] [PATCH] qemu: domain: Fix potential NULL deref when parsing job private data

[Erik Skultety]

On Mon, Sep 02, 2019 at 04:13:55PM +0200, Peter Krempa wrote:
> A specially crafted XML which would reference a non-existing disk but
> request the mirror to be registered with the blockjob could potentially
> make the parser dereference NULL. Fix it by moving the code slightly and
> just treat it as a wrong job XML. Found by Coverity.
>
> Reported-by: John Ferlan 
> Signed-off-by: Peter Krempa 
> ---
Reviewed-by: Erik Skultety 

safe for freeze

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

Re: [libvirt] [PATCH 7/7] util: use glib allocation functions

[Daniel P . Berrangé]

On Mon, Sep 02, 2019 at 04:52:47PM +0200, Ján Tomko wrote:
> On Thu, Aug 29, 2019 at 07:02:50PM +0100, Daniel P. Berrangé wrote:
> > GLib requires that any memory allocated with g_new/g_malloc/etc
> > is free'd by g_free. This means mixing virAlloc with g_free,
> > or g_new with virFree will violate API rules. The easy way to
> > dea with this is to simply make our allocation functions wrappers
> 
> s/dea/deal/
> 
> > to the glib functions.
> > 
> > Use of our allocation functions can be phased out gradually
> > over time. When doing such conversions, the return values checks
> > can be dropped at the same time.
> > 
> > Signed-off-by: Daniel P. Berrangé 
> > ---
> > src/util/viralloc.c | 29 ++---
> > 1 file changed, 6 insertions(+), 23 deletions(-)
> > 
> 
> I'm afraid this is incomplete.
> 
> The obvious mismatch being VIR_STRDUP calling strdup while
> VIR_FREE would call g_free now.

Opps, yes.

> Same with (v)asprintf. We also use VIR_FREE to free memory
> allocated by external libraries (e.g. virXMLPropString; while
> there is a xmlFree function which is just an alias for free,
> we never use it in libvirt code)

Hmm, not using xmlFree is arguably a bug in libvirt code.

Most other libs quite probably mandate plain 'free'.

> Maybe the safer approach would be to convert the code gradually
> and explicitly use g_alloc and g_free/g_autoptr without the wrappers?

I'm not convince that would be especially safe - the caller of any
API which returns an allocated string now checks to look at the impl
to see how it was allocated - with deep call chains this gets quite
error prone I think.

What's interesting is that the glib docs says

 * It's important to match g_malloc() (and wrappers such as g_new()) with
 * g_free(), g_slice_alloc() (and wrappers such as g_slice_new()) with
 * g_slice_free(), plain malloc() with free(), and (if you're using C++)
 * new with delete and new[] with delete[]. Otherwise bad things can happen,
 * since these allocators may use different memory pools (and new/delete call
 * constructors and destructors).

but the actual code just does

gpointer
g_malloc (gsize n_bytes)
{
  if (G_LIKELY (n_bytes))
{
  gpointer mem;

  mem = malloc (n_bytes);
  TRACE (GLIB_MEM_ALLOC((void*) mem, (unsigned int) n_bytes, 0, 0));
  if (mem)
return mem;

  g_error ("%s: failed to allocate %"G_GSIZE_FORMAT" bytes",
   G_STRLOC, n_bytes);
}

  TRACE(GLIB_MEM_ALLOC((void*) NULL, (int) n_bytes, 0, 0));

  return NULL;
}

void
g_free (gpointer mem)
{
  free (mem);
  TRACE(GLIB_MEM_FREE((void*) mem));
}

IOW, it is entirely safe to mix free + g_free today - looks like the warning
is just talking about a hypothetical risk that doesn't actually exist right
now, at least for free vs g_free.

The g_slice warning is definitely relevant though since that uses a pool
allocator

So I'm inclined to fix the mistakes in this patch, and then just deal with
using plain 'free' for any cases we deal with external libraries on best
effort, in the belief that it doesnt' actually matter for glib anyway.

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 7/7] util: use glib allocation functions

[Ján Tomko]

On Thu, Aug 29, 2019 at 07:02:50PM +0100, Daniel P. Berrangé wrote:

GLib requires that any memory allocated with g_new/g_malloc/etc
is free'd by g_free. This means mixing virAlloc with g_free,
or g_new with virFree will violate API rules. The easy way to
dea with this is to simply make our allocation functions wrappers


s/dea/deal/


to the glib functions.

Use of our allocation functions can be phased out gradually
over time. When doing such conversions, the return values checks
can be dropped at the same time.

Signed-off-by: Daniel P. Berrangé 
---
src/util/viralloc.c | 29 ++---
1 file changed, 6 insertions(+), 23 deletions(-)



I'm afraid this is incomplete.

The obvious mismatch being VIR_STRDUP calling strdup while
VIR_FREE would call g_free now.

Same with (v)asprintf. We also use VIR_FREE to free memory
allocated by external libraries (e.g. virXMLPropString; while
there is a xmlFree function which is just an alias for free,
we never use it in libvirt code)

Maybe the safer approach would be to convert the code gradually
and explicitly use g_alloc and g_free/g_autoptr without the wrappers?

Jano


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

Re: [libvirt] [PATCH] qemu: domain: Fix potential NULL deref when parsing job private data

[John Ferlan]

On 9/2/19 10:13 AM, Peter Krempa wrote:
> A specially crafted XML which would reference a non-existing disk but
> request the mirror to be registered with the blockjob could potentially
> make the parser dereference NULL. Fix it by moving the code slightly and
> just treat it as a wrong job XML. Found by Coverity.
> 
> Reported-by: John Ferlan 
> Signed-off-by: Peter Krempa 
> ---
>  src/qemu/qemu_domain.c | 10 +++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 

Reviewed-by: John Ferlan 

SFF

John

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

Re: [libvirt] [PATCH v2 10/10] tests: Introduce resolution test for VGA model

[Julio Faracco]

This patch is missing XML files for VGA test case.
I will wait for review until submit a V3.

--
Julio Cesar Faracco




Em sex, 30 de ago de 2019 às 18:43,  escreveu:
>
> From: Julio Faracco 
>
> New test case was added to cover 'xres' and 'yres' properties for VGA
> model due to latest QEMU version.
>
> Signed-off-by: Julio Faracco 
> ---
>  tests/qemuxml2argvtest.c | 1 +
>  tests/qemuxml2xmltest.c  | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index f68c31d5e9..66b63925d2 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -1987,6 +1987,7 @@ mymain(void)
>  QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
>  DO_TEST("video-vga-device-vgamem", QEMU_CAPS_DEVICE_VGA,
>  QEMU_CAPS_DEVICE_VIDEO_PRIMARY, QEMU_CAPS_VGA_VGAMEM);
> +DO_TEST_CAPS_LATEST("video-vga-resolution");
>  DO_TEST("video-qxl-nodevice", QEMU_CAPS_DEVICE_QXL);
>  DO_TEST("video-qxl-device",
>  QEMU_CAPS_DEVICE_QXL,
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index 3078355019..ac537767ba 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -1200,6 +1200,7 @@ mymain(void)
>
>  DO_TEST("acpi-table", NONE);
>
> +DO_TEST_CAPS_LATEST("video-vga-resolution");
>  DO_TEST("video-device-pciaddr-default",
>  QEMU_CAPS_KVM,
>  QEMU_CAPS_VNC,
> --
> 2.20.1
>

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

[libvirt] [PATCH] util: Set backing file name for LOOP_GET_STATUS64 queries.

[jcfaracco]

From: Julio Faracco 

This is an issue for LXC loop devices when you are trying to get loop
devices info using `ioctl`. Modern apps uses `/sys/dev/block` to grab
information about devices, but if you use the method mention you won't
be able to retrive the associated file with that loop device. See
example below from cryptsetup sources:

static char *_ioctl_backing_file(const char *loop)
{
struct loop_info64 lo64 = {0};
int loop_fd;

loop_fd = open(loop, O_RDONLY);
if (loop_fd < 0)
return NULL;

if (ioctl(loop_fd, LOOP_GET_STATUS64, ) < 0) {
close(loop_fd);
return NULL;
}

lo64.lo_file_name[LO_NAME_SIZE-2] = '*';
lo64.lo_file_name[LO_NAME_SIZE-1] = 0;

close(loop_fd);
return strdup((char*)lo64.lo_file_name);
}

It will return an empty string because lo_file_name was not set.

Signed-off-by: Julio Faracco 
---
 src/util/virfile.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 81a3c096eb..fe0aa54eb6 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -781,6 +781,16 @@ int virFileLoopDeviceAssociate(const char *file,
 memset(, 0, sizeof(lo));
 lo.lo_flags = LO_FLAGS_AUTOCLEAR;
 
+/* Set backing file name for LOOP_GET_STATUS64 queries */
+if (virStrncpy((char *) lo.lo_file_name, file,
+   strlen(file), LO_NAME_SIZE) < 0) {
+virReportSystemError(errno,
+ _("Unable to set backing file %s"), file);
+goto cleanup;
+}
+lo.lo_file_name[LO_NAME_SIZE-2] = '*';
+lo.lo_file_name[LO_NAME_SIZE-1] = 0;
+
 if ((fsfd = open(file, O_RDWR)) < 0) {
 virReportSystemError(errno,
  _("Unable to open %s"), file);
-- 
2.20.1

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

Re: [libvirt] [PATCH 5/7] build: probe for glib-2 library in configure

[Daniel P . Berrangé]

On Mon, Sep 02, 2019 at 04:05:12PM +0200, Ján Tomko wrote:
> On Thu, Aug 29, 2019 at 07:02:48PM +0100, Daniel P. Berrangé wrote:
> > Prepare for linking with glib by probing for it at configure
> > time. Per supported platforms target, the min glib versions on
> > relevant distros are:
> > 
> >  RHEL-8: 2.56.1
> >  RHEL-7: 2.50.3
> >  Debian (Buster): 2.58.3
> >  Debian (Stretch): 2.50.3
> >  OpenBSD (Ports): 2.58.3
> >  FreeBSD (Ports): 2.56.3
> >  OpenSUSE Leap 15: 2.54.3
> >  SLE12-SP2: 2.48.2
> >  Ubuntu (Xenial): 2.48.0
> >  macOS (Homebrew): 2.56.0
> > 
> > This suggests that a minimum glib of 2.48 is a reasonable target.
> > 
> 
> Note that CentOS 6 has 2.28.8
> 
> > Signed-off-by: Daniel P. Berrangé 
> > ---
> > configure.ac  |  2 ++
> > libvirt.spec.in   |  1 +
> > m4/virt-glib.m4   | 30 ++
> > mingw-libvirt.spec.in |  2 ++
> > 4 files changed, 35 insertions(+)
> > create mode 100644 m4/virt-glib.m4
> > 
> > diff --git a/m4/virt-glib.m4 b/m4/virt-glib.m4
> > new file mode 100644
> > index 00..9c7acb7889
> > --- /dev/null
> > +++ b/m4/virt-glib.m4
> > @@ -0,0 +1,30 @@
> > +dnl The glib.so library
> > +dnl
> > +dnl Copyright (C) 2016 Red Hat, Inc.
> > +dnl
> > +dnl This library is free software; you can redistribute it and/or
> > +dnl modify it under the terms of the GNU Lesser General Public
> > +dnl License as published by the Free Software Foundation; either
> > +dnl version 2.1 of the License, or (at your option) any later version.
> > +dnl
> > +dnl This library is distributed in the hope that it will be useful,
> > +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +dnl Lesser General Public License for more details.
> > +dnl
> > +dnl You should have received a copy of the GNU Lesser General Public
> > +dnl License along with this library.  If not, see
> > +dnl .
> > +dnl
> > +
> > +AC_DEFUN([LIBVIRT_ARG_GLIB], [
> > +  LIBVIRT_ARG_WITH([GLIB], [glib-2.0 location], [check])
> > +])
> > +
> > +AC_DEFUN([LIBVIRT_CHECK_GLIB],[
> > +  LIBVIRT_CHECK_PKG([GLIB], [gthread-2.0], [2.48.0])
> 
> Given that pretty much everything requires us to allocate memory,
> failing to find it should be fatal.

Opps, yes, forgot this macro isn't fatal.

> (Which OTOH would block even docs generation, which should not need C
> code to be run)

libvirt.org docs generation is already doomed due to the recent
libxml2 min version update, so I'm already working to fix that.

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 6/7] build: link to glib library

[Ján Tomko]

On Thu, Aug 29, 2019 at 07:02:49PM +0100, Daniel P. Berrangé wrote:

Add the main glib.h to internal.h so that all common code can use it.

Signed-off-by: Daniel P. Berrangé 
---
src/Makefile.am| 2 ++
src/internal.h | 1 +
src/lxc/Makefile.inc.am| 2 ++
src/remote/Makefile.inc.am | 1 +
src/util/Makefile.inc.am   | 1 +
tests/Makefile.am  | 3 ++-
tools/Makefile.am  | 1 +
7 files changed, 10 insertions(+), 1 deletion(-)



Reviewed-by: Ján Tomko 

Jano


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

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

[Jim Fehlig]

On 9/2/19 8:16 AM, Daniel P. Berrangé  wrote:
> On Mon, Sep 02, 2019 at 02:07:08PM +, Jim Fehlig wrote:
>> On 8/31/19 2:11 AM, Michal Prívozník  wrote:
>>> On 8/26/19 1:49 PM, Ján Tomko wrote:
 The 'From:' field shows your e-mail in uppercase.

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

>>>
>>> Actually, this breaks --with-xenapi build:
>>>
>>> xenapi/xenapi_driver.c:38:10: fatal error: xen_common.h: No such file or
>>> directory
>>>#include "xen_common.h"
>>> ^~
>>> compilation terminated.
>>
>> Can you determine why this is needed? E.g. remove it and see what 
>> subsequently
>> fails? I don't have a setup readily available to test a xenapi build (I'm not
>> "in the office" today as it is a US holiday).
> 
> I'm inclined to say that we should call the XenAPI driver dead at
> this point.

As an alternative to fixing the bugld, I was just about to suggest the same! 
It's a drastic fix: kill the driver :-). I can work on this, perhaps in the 
evening. It's probably not much effort. The driver is not even mentioned in the 
drivers page

https://libvirt.org/drivers.html

Regards,
Jim

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

Re: [libvirt] [RFC] On present using dummy hostdev usb device

[Daniel P . Berrangé]

On Mon, Sep 02, 2019 at 02:11:01PM +, Roman Kagan wrote:
> On Fri, Aug 30, 2019 at 04:19:23PM +0100, Daniel P. Berrangé wrote:
> > On Fri, Aug 30, 2019 at 05:08:52PM +0200, Michal Privoznik wrote:
> > > On 8/30/19 2:30 PM, Roman Kagan wrote:
> > > > On Fri, Aug 30, 2019 at 10:09:06AM +0100, Daniel P. Berrangé wrote:
> > > > > On Fri, Aug 30, 2019 at 08:44:03AM +, Nikolay Shirokovskiy wrote:
> > > > > > Hi, all!
> > > > > > 
> > > > > >We use an interesting approach when starting/migrating/etc 
> > > > > > domain with usb
> > > > > > hostdev with startupPolicy=optional. We add qemu usb-host device 
> > > > > > with
> > > > > > missing hostaddr/hostbus parameters (dummy device). I guess there 
> > > > > > are
> > > > > > 2 reasons why we do it. First without dummy device migration will 
> > > > > > fail as
> > > > > > described in [1]. Second is an interesting property of dummy device 
> > > > > > that
> > > > > > qemu starts to monitor for attaching of usb devices and binds the 
> > > > > > first
> > > > > > attached to node to the dummy device. So one can start a domain with
> > > > > > missing hostdev and attach it later or migrate a domain then detach
> > > > > > hostdev on source and attach it on destination. But as qemu binds 
> > > > > > the
> > > > > > first attached device this is not reliable, to say the least. And 
> > > > > > after
> > > > > > all this does not work if domain uses distinct mount namespace which
> > > > > > is default.
> > > > > 
> > > > > Even without mount namespaces, it should fail as QEMU is running  
> > > > > non-root
> > > > > and libvirt won't have granted access to any host USB devices in 
> > > > > /dev, and
> > > > > also SELinux policy will forbid this.
> > > > 
> > > > Right, but the case with mount namespaces is particularly problematic:
> > > > if the device open fails due to missing device node, libusb removes the
> > > > device from its internal device list.  This results in the following
> > > > scenario:
> > > > 
> > > > - libvirt adds a dummy usb-host device to QEMU in place of a missing
> > > >device
> > > > 
> > > > - QEMU (via libusb) installs a watch for udev add events
> > > > 
> > > > - the physical device is plugged into the host
> > > > 
> > > > - QEMU detects the addition of the device and, since the dummy device
> > > >matches everything, tries to open it
> > > > 
> > > > - by this time libvirt may have not created a device node in QEMU's
> > > >mount namespace, so the open fails due to missing device node, and
> > > >libusb removes the device from its internal list
> > > > 
> > > > - libvirt removes the dummy usb-host device and adds the actual usb-host
> > > >device
> > > > 
> > > > - QEMU fails to open it because it's no longer seen by libusb
> > > 
> > > There is a bug filed against libusb exactly for this:
> > > 
> > > https://bugzilla.redhat.com/show_bug.cgi?id=1595525
> > > 
> > > BTW: you don't have to migrate, it's sufficient to start a domain with a
> > > missing USB and startupPolicy='optional' and then physically plug it into
> > > the host and then try to hotplug it into the domain.
> > 
> > AFAIR, the startupPolicy=optional was never really intended to allow
> > for the device to be dynamically added after startup. It was only
> > trying drop the device if it didn't exist at startup.
> > 
> > If we do want a way to dynamically add after startup, then libvirt itself
> > would have to monitor the USB devices on the host, and then perform QMP
> > commands to hot-add to QEMU.
> 
> This is actually what Nikolay was trying to achieve with his patchset
> (https://www.redhat.com/archives/libvir-list/2019-August/msg01413.html).
> Is this a NAK to it, with a suggestion that it's the upper layer's
> responsibility?

I think so. The error handling around hostdevs and migration has some
quite nasty edge cases. In particular if you unplug before migrating
and migrate fails, and you try to restart the source VM, but then have
failures re-adding the USB devices.  The mgmt app has to interogate
libvirt to see what USB devices were succesfully attached and which
failed, so they can account for their usage. At that point you might
as well just have the mgmt app own the whole process IMHO.

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] xenconfig: move contents to libxl driver and remove directory

[Daniel P . Berrangé]

On Mon, Sep 02, 2019 at 02:07:08PM +, Jim Fehlig wrote:
> On 8/31/19 2:11 AM, Michal Prívozník  wrote:
> > On 8/26/19 1:49 PM, Ján Tomko wrote:
> >> The 'From:' field shows your e-mail in uppercase.
> >>
> >> On Fri, Aug 23, 2019 at 07:50:12PM +, Jim Fehlig wrote:
> >>> Signed-off-by: Jim Fehlig 
> >>> ---
> >>> cfg.mk   |  2 +-
> >>> configure.ac |  2 --
> >>> po/POTFILES  |  6 ++---
> >>> src/Makefile.am  |  1 -
> >>> src/libvirt_xenconfig.syms   | 12 --
> >>> src/libxl/Makefile.inc.am    | 25 ++---
> >>> src/{xenconfig => libxl}/xen_common.c    |  0
> >>> src/{xenconfig => libxl}/xen_common.h    |  0
> >>> src/{xenconfig => libxl}/xen_xl.c    |  0
> >>> src/{xenconfig => libxl}/xen_xl.h    |  0
> >>> src/{xenconfig => libxl}/xen_xm.c    |  0
> >>> src/{xenconfig => libxl}/xen_xm.h    |  0
> >>> src/{xenconfig => libxl}/xenxs_private.h |  0
> >>> src/xenconfig/Makefile.inc.am    | 28 
> >>> tests/xlconfigtest.c |  2 +-
> >>> tests/xmconfigtest.c |  2 +-
> >>> 16 files changed, 13 insertions(+), 67 deletions(-)
> >>>
> >>
> > 
> > Actually, this breaks --with-xenapi build:
> > 
> > xenapi/xenapi_driver.c:38:10: fatal error: xen_common.h: No such file or
> > directory
> >   #include "xen_common.h"
> >^~
> > compilation terminated.
> 
> Can you determine why this is needed? E.g. remove it and see what 
> subsequently 
> fails? I don't have a setup readily available to test a xenapi build (I'm not 
> "in the office" today as it is a US holiday).

I'm inclined to say that we should call the XenAPI driver dead at
this point. IIUC, the person at Citrix who wrote left, left that
job shortly after it was merged into libvirt. Looking at the git
history I'm struggling to see any single patch applied in 9 years
since that was not a general cleanup/bugfix. The XML handling does
not deal with disks at all AFAICT making it largely useless.

IOW, lets just document that it fails to build for this release
and then delete its code.

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

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

[libvirt] [PATCH] qemu: domain: Fix potential NULL deref when parsing job private data

[Peter Krempa]

A specially crafted XML which would reference a non-existing disk but
request the mirror to be registered with the blockjob could potentially
make the parser dereference NULL. Fix it by moving the code slightly and
just treat it as a wrong job XML. Found by Coverity.

Reported-by: John Ferlan 
Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_domain.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 657f3ecfe4..c7eb0b5e9a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3012,15 +3012,19 @@ 
qemuDomainObjPrivateXMLParseBlockjobData(virDomainObjPtr vm,
 invalidData = true;
 }

+if (mirror) {
+if (disk)
+job->mirrorChain = virObjectRef(disk->mirror);
+else
+invalidData = true;
+}
+
 job->state = state;
 job->newstate = newstate;
 job->errmsg = virXPathString("string(./errmsg)", ctxt);
 job->invalidData = invalidData;
 job->disk = disk;

-if (mirror)
-job->mirrorChain = virObjectRef(job->disk->mirror);
-
 qemuDomainObjPrivateXMLParseBlockjobDataSpecific(job, ctxt, xmlopt);

 if (qemuBlockJobRegister(job, vm, disk, false) < 0)
-- 
2.21.0

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

Re: [libvirt] [RFC] On present using dummy hostdev usb device

[Roman Kagan]

On Fri, Aug 30, 2019 at 04:19:23PM +0100, Daniel P. Berrangé wrote:
> On Fri, Aug 30, 2019 at 05:08:52PM +0200, Michal Privoznik wrote:
> > On 8/30/19 2:30 PM, Roman Kagan wrote:
> > > On Fri, Aug 30, 2019 at 10:09:06AM +0100, Daniel P. Berrangé wrote:
> > > > On Fri, Aug 30, 2019 at 08:44:03AM +, Nikolay Shirokovskiy wrote:
> > > > > Hi, all!
> > > > > 
> > > > >We use an interesting approach when starting/migrating/etc domain 
> > > > > with usb
> > > > > hostdev with startupPolicy=optional. We add qemu usb-host device with
> > > > > missing hostaddr/hostbus parameters (dummy device). I guess there are
> > > > > 2 reasons why we do it. First without dummy device migration will 
> > > > > fail as
> > > > > described in [1]. Second is an interesting property of dummy device 
> > > > > that
> > > > > qemu starts to monitor for attaching of usb devices and binds the 
> > > > > first
> > > > > attached to node to the dummy device. So one can start a domain with
> > > > > missing hostdev and attach it later or migrate a domain then detach
> > > > > hostdev on source and attach it on destination. But as qemu binds the
> > > > > first attached device this is not reliable, to say the least. And 
> > > > > after
> > > > > all this does not work if domain uses distinct mount namespace which
> > > > > is default.
> > > > 
> > > > Even without mount namespaces, it should fail as QEMU is running  
> > > > non-root
> > > > and libvirt won't have granted access to any host USB devices in /dev, 
> > > > and
> > > > also SELinux policy will forbid this.
> > > 
> > > Right, but the case with mount namespaces is particularly problematic:
> > > if the device open fails due to missing device node, libusb removes the
> > > device from its internal device list.  This results in the following
> > > scenario:
> > > 
> > > - libvirt adds a dummy usb-host device to QEMU in place of a missing
> > >device
> > > 
> > > - QEMU (via libusb) installs a watch for udev add events
> > > 
> > > - the physical device is plugged into the host
> > > 
> > > - QEMU detects the addition of the device and, since the dummy device
> > >matches everything, tries to open it
> > > 
> > > - by this time libvirt may have not created a device node in QEMU's
> > >mount namespace, so the open fails due to missing device node, and
> > >libusb removes the device from its internal list
> > > 
> > > - libvirt removes the dummy usb-host device and adds the actual usb-host
> > >device
> > > 
> > > - QEMU fails to open it because it's no longer seen by libusb
> > 
> > There is a bug filed against libusb exactly for this:
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1595525
> > 
> > BTW: you don't have to migrate, it's sufficient to start a domain with a
> > missing USB and startupPolicy='optional' and then physically plug it into
> > the host and then try to hotplug it into the domain.
> 
> AFAIR, the startupPolicy=optional was never really intended to allow
> for the device to be dynamically added after startup. It was only
> trying drop the device if it didn't exist at startup.
> 
> If we do want a way to dynamically add after startup, then libvirt itself
> would have to monitor the USB devices on the host, and then perform QMP
> commands to hot-add to QEMU.

This is actually what Nikolay was trying to achieve with his patchset
(https://www.redhat.com/archives/libvir-list/2019-August/msg01413.html).
Is this a NAK to it, with a suggestion that it's the upper layer's
responsibility?

Thanks,
Roman.

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

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

[Jim Fehlig]

On 8/31/19 2:11 AM, Michal Prívozník  wrote:
> On 8/26/19 1:49 PM, Ján Tomko wrote:
>> The 'From:' field shows your e-mail in uppercase.
>>
>> On Fri, Aug 23, 2019 at 07:50:12PM +, Jim Fehlig wrote:
>>> Signed-off-by: Jim Fehlig 
>>> ---
>>> cfg.mk   |  2 +-
>>> configure.ac |  2 --
>>> po/POTFILES  |  6 ++---
>>> src/Makefile.am  |  1 -
>>> src/libvirt_xenconfig.syms   | 12 --
>>> src/libxl/Makefile.inc.am    | 25 ++---
>>> src/{xenconfig => libxl}/xen_common.c    |  0
>>> src/{xenconfig => libxl}/xen_common.h    |  0
>>> src/{xenconfig => libxl}/xen_xl.c    |  0
>>> src/{xenconfig => libxl}/xen_xl.h    |  0
>>> src/{xenconfig => libxl}/xen_xm.c    |  0
>>> src/{xenconfig => libxl}/xen_xm.h    |  0
>>> src/{xenconfig => libxl}/xenxs_private.h |  0
>>> src/xenconfig/Makefile.inc.am    | 28 
>>> tests/xlconfigtest.c |  2 +-
>>> tests/xmconfigtest.c |  2 +-
>>> 16 files changed, 13 insertions(+), 67 deletions(-)
>>>
>>
> 
> Actually, this breaks --with-xenapi build:
> 
> xenapi/xenapi_driver.c:38:10: fatal error: xen_common.h: No such file or
> directory
>   #include "xen_common.h"
>^~
> compilation terminated.

Can you determine why this is needed? E.g. remove it and see what subsequently 
fails? I don't have a setup readily available to test a xenapi build (I'm not 
"in the office" today as it is a US holiday).

Regards,
Jim

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

Re: [libvirt] [PATCH 5/7] build: probe for glib-2 library in configure

[Ján Tomko]

On Thu, Aug 29, 2019 at 07:02:48PM +0100, Daniel P. Berrangé wrote:

Prepare for linking with glib by probing for it at configure
time. Per supported platforms target, the min glib versions on
relevant distros are:

 RHEL-8: 2.56.1
 RHEL-7: 2.50.3
 Debian (Buster): 2.58.3
 Debian (Stretch): 2.50.3
 OpenBSD (Ports): 2.58.3
 FreeBSD (Ports): 2.56.3
 OpenSUSE Leap 15: 2.54.3
 SLE12-SP2: 2.48.2
 Ubuntu (Xenial): 2.48.0
 macOS (Homebrew): 2.56.0

This suggests that a minimum glib of 2.48 is a reasonable target.



Note that CentOS 6 has 2.28.8


Signed-off-by: Daniel P. Berrangé 
---
configure.ac  |  2 ++
libvirt.spec.in   |  1 +
m4/virt-glib.m4   | 30 ++
mingw-libvirt.spec.in |  2 ++
4 files changed, 35 insertions(+)
create mode 100644 m4/virt-glib.m4

diff --git a/m4/virt-glib.m4 b/m4/virt-glib.m4
new file mode 100644
index 00..9c7acb7889
--- /dev/null
+++ b/m4/virt-glib.m4
@@ -0,0 +1,30 @@
+dnl The glib.so library
+dnl
+dnl Copyright (C) 2016 Red Hat, Inc.
+dnl
+dnl This library is free software; you can redistribute it and/or
+dnl modify it under the terms of the GNU Lesser General Public
+dnl License as published by the Free Software Foundation; either
+dnl version 2.1 of the License, or (at your option) any later version.
+dnl
+dnl This library is distributed in the hope that it will be useful,
+dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
+dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+dnl Lesser General Public License for more details.
+dnl
+dnl You should have received a copy of the GNU Lesser General Public
+dnl License along with this library.  If not, see
+dnl .
+dnl
+
+AC_DEFUN([LIBVIRT_ARG_GLIB], [
+  LIBVIRT_ARG_WITH([GLIB], [glib-2.0 location], [check])
+])
+
+AC_DEFUN([LIBVIRT_CHECK_GLIB],[
+  LIBVIRT_CHECK_PKG([GLIB], [gthread-2.0], [2.48.0])


Given that pretty much everything requires us to allocate memory,
failing to find it should be fatal.

(Which OTOH would block even docs generation, which should not need C
code to be run)

Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH v4 2/2] snapshot: Store both config and live XML in the snapshot domain

[Daniel Henrique Barboza]

On 8/29/19 5:55 PM, Maxiwell S. Garcia wrote:

The snapshot-create operation of running guests saves the live
XML and uses it to replace the active and inactive domain in
case of revert. So, the config XML is ignored by the snapshot
process. This commit changes it and adds the config XML in the
snapshot XML as the  entry.

In case of offline guest, the behavior remains the same and the
config XML is saved in the snapshot XML as  entry. The
behavior of older snapshots of running guests, that don't have
the new , remains the same too. The revert, in
this case, overrides both active and inactive domain with the
 entry. So, the  in the snapshot XML is
not required to snapshot work, but it's useful to preserve the
config XML of running guests.

Signed-off-by: Maxiwell S. Garcia 
---


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


  src/conf/moment_conf.c   |  1 +
  src/conf/moment_conf.h   | 11 +++
  src/conf/snapshot_conf.c | 22 --
  src/qemu/qemu_driver.c   | 37 -
  4 files changed, 60 insertions(+), 11 deletions(-)

diff --git a/src/conf/moment_conf.c b/src/conf/moment_conf.c
index fea13f0f97..f54a44b33e 100644
--- a/src/conf/moment_conf.c
+++ b/src/conf/moment_conf.c
@@ -66,6 +66,7 @@ virDomainMomentDefDispose(void *obj)
  VIR_FREE(def->description);
  VIR_FREE(def->parent_name);
  virDomainDefFree(def->dom);
+virDomainDefFree(def->inactiveDom);
  }
  
  /* Provide defaults for creation time and moment name after parsing XML */

diff --git a/src/conf/moment_conf.h b/src/conf/moment_conf.h
index 9fdbef2172..70cc47bd70 100644
--- a/src/conf/moment_conf.h
+++ b/src/conf/moment_conf.h
@@ -36,7 +36,18 @@ struct _virDomainMomentDef {
  char *parent_name;
  long long creationTime; /* in seconds */
  
+/*

+ * Store the active domain definition in case of online
+ * guest and the inactive domain definition in case of
+ * offline guest
+ */
  virDomainDefPtr dom;
+
+/*
+ * Store the inactive domain definition in case of online
+ * guest and leave NULL in case of offline guest
+ */
+virDomainDefPtr inactiveDom;
  };
  
  virClassPtr virClassForDomainMomentDef(void);

diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index 7996589ad2..cce9a7999c 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -235,6 +235,7 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
  virDomainSnapshotDefPtr def = NULL;
  virDomainSnapshotDefPtr ret = NULL;
  xmlNodePtr *nodes = NULL;
+xmlNodePtr inactiveDomNode = NULL;
  size_t i;
  int n;
  char *creation = NULL, *state = NULL;
@@ -244,6 +245,8 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
  char *memoryFile = NULL;
  bool offline = !!(flags & VIR_DOMAIN_SNAPSHOT_PARSE_OFFLINE);
  virSaveCookieCallbacksPtr saveCookie = 
virDomainXMLOptionGetSaveCookie(xmlopt);
+int domainflags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
+  VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;
  
  if (!(def = virDomainSnapshotDefNew()))

  return NULL;
@@ -293,8 +296,6 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
   * clients will have to decide between best effort
   * initialization or outright failure.  */
  if ((tmp = virXPathString("string(./domain/@type)", ctxt))) {
-int domainflags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
-  VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;
  xmlNodePtr domainNode = virXPathNode("./domain", ctxt);
  
  VIR_FREE(tmp);

@@ -311,6 +312,16 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
  } else {
  VIR_WARN("parsing older snapshot that lacks domain");
  }
+
+/* /inactiveDomain entry saves the config XML present in a running
+ * VM. In case of absent, leave parent.inactiveDom NULL and use
+ * parent.dom for config and live XML. */
+if ((inactiveDomNode = virXPathNode("./inactiveDomain", ctxt))) {
+def->parent.inactiveDom = virDomainDefParseNode(ctxt->node->doc, 
inactiveDomNode,
+caps, xmlopt, 
NULL, domainflags);
+if (!def->parent.inactiveDom)
+goto cleanup;
+}
  } else if (virDomainXMLOptionRunMomentPostParse(xmlopt, >parent) < 
0) {
  goto cleanup;
  }
@@ -908,6 +919,13 @@ virDomainSnapshotDefFormatInternal(virBufferPtr buf,
  virBufferAddLit(buf, "\n");
  }
  
+if (def->parent.inactiveDom) {

+if (virDomainDefFormatInternalSetRootName(def->parent.inactiveDom, 
caps,
+  domainflags, buf, xmlopt,
+  "inactiveDomain") < 0)
+goto error;
+}
+
  if (virSaveCookieFormatBuf(buf, def->cookie,

Re: [libvirt] [PATCH v4 1/2] qemu: formatting XML from domain def choosing the root name

[Daniel Henrique Barboza]

On 8/29/19 5:55 PM, Maxiwell S. Garcia wrote:

The function virDomainDefFormatInternal() has the predefined root name
"domain" to format the XML. But to save both active and inactive domain
in the snapshot XML, the new root name "inactiveDomain" was created.
So, the new function virDomainDefFormatInternalSetRootName() allows to
choose the root name of XML. The former function became a tiny wrapper
to call the new function setting the correct parameters.

Signed-off-by: Maxiwell S. Garcia 
---


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



  src/conf/domain_conf.c | 35 ++-
  src/conf/domain_conf.h |  6 ++
  2 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b7a342bb91..3154c07a86 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -21517,10 +21517,11 @@ virDomainDefParseNode(xmlDocPtr xml,
  virDomainDefPtr def = NULL;
  virDomainDefPtr ret = NULL;
  
-if (!virXMLNodeNameEqual(root, "domain")) {

+if ((!virXMLNodeNameEqual(root, "domain")) &&
+(!virXMLNodeNameEqual(root, "inactiveDomain"))) {
  virReportError(VIR_ERR_XML_ERROR,
 _("unexpected root element <%s>, "
- "expecting "),
+ "expecting  or "),
 root->name);
  goto cleanup;
  }
@@ -28277,17 +28278,29 @@ virDomainDefFormatFeatures(virBufferPtr buf,
  return virXMLFormatElement(buf, "features", NULL, );
  }
  
-

-/* This internal version appends to an existing buffer
- * (possibly with auto-indent), rather than flattening
- * to string.
- * Return -1 on failure.  */
  int
  virDomainDefFormatInternal(virDomainDefPtr def,
 virCapsPtr caps,
 unsigned int flags,
 virBufferPtr buf,
 virDomainXMLOptionPtr xmlopt)
+{
+return virDomainDefFormatInternalSetRootName(def, caps, flags, buf,
+ xmlopt, "domain");
+}
+
+
+/* This internal version appends to an existing buffer
+ * (possibly with auto-indent), rather than flattening
+ * to string.
+ * Return -1 on failure.  */
+int
+virDomainDefFormatInternalSetRootName(virDomainDefPtr def,
+  virCapsPtr caps,
+  unsigned int flags,
+  virBufferPtr buf,
+  virDomainXMLOptionPtr xmlopt,
+  const char *rootname)
  {
  unsigned char *uuid;
  char uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -28312,7 +28325,11 @@ virDomainDefFormatInternal(virDomainDefPtr def,
  if (def->id == -1)
  flags |= VIR_DOMAIN_DEF_FORMAT_INACTIVE;
  
-virBufferAsprintf(buf, "
+if (!rootname || (STRNEQ(rootname, "domain") &&
+  STRNEQ(rootname, "inactiveDomain")))
+goto error;
+
+virBufferAsprintf(buf, "<%s type='%s'", rootname, type);
  if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))
  virBufferAsprintf(buf, " id='%d'", def->id);
  if (def->namespaceData && def->ns.format)
@@ -28794,7 +28811,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
  virDomainSEVDefFormat(buf, def->sev);
  
  virBufferAdjustIndent(buf, -2);

-virBufferAddLit(buf, "\n");
+virBufferAsprintf(buf, "\n", rootname);
  
  if (virBufferCheckError(buf) < 0)

  goto error;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 33cef5b75c..af1335cc16 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3073,6 +3073,12 @@ int virDomainDefFormatInternal(virDomainDefPtr def,
 unsigned int flags,
 virBufferPtr buf,
 virDomainXMLOptionPtr xmlopt);
+int virDomainDefFormatInternalSetRootName(virDomainDefPtr def,
+  virCapsPtr caps,
+  unsigned int flags,
+  virBufferPtr buf,
+  virDomainXMLOptionPtr xmlopt,
+  const char *rootname);
  
  int virDomainDiskSourceFormat(virBufferPtr buf,

virStorageSourcePtr src,


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

Re: [libvirt] [PATCH 4/7] util: make string functions abort on OOM

[Ján Tomko]

On Thu, Aug 29, 2019 at 07:02:47PM +0100, Daniel P. Berrangé wrote:

The functions are left returning an "int" to avoid an immediate
big-bang cleanup. They'll simply never return anything other
than 0.

Signed-off-by: Daniel P. Berrangé 
---
src/util/virstring.c | 93 +++-
src/util/virstring.h | 79 +
2 files changed, 49 insertions(+), 123 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 3/7] util: remove several unused _QUIET allocation macro variants

[Ján Tomko]

On Thu, Aug 29, 2019 at 07:02:46PM +0100, Daniel P. Berrangé wrote:

Only a few of the _QUIET allocation macros are used. Since we're no
longer reporting OOM as errors, we want to eliminate all the _QUIET
variants. This starts with the easy, unused, cases.

Signed-off-by: Daniel P. Berrangé 
---
src/util/viralloc.h | 74 -
1 file changed, 74 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 2/7] util: make allocation functions abort on OOM

[Ján Tomko]

On Thu, Aug 29, 2019 at 07:02:45PM +0100, Daniel P. Berrangé wrote:

The functions are left returning an "int" to avoid an immediate
big-bang cleanup. They'll simply never return anything other
than 0, except for virInsertN which can still return an error
if the requested insertion index is out of range. Interestingly
in that case, the _QUIET function would none the less report
an error.

Signed-off-by: Daniel P. Berrangé 
---
src/util/viralloc.c | 201 +++-
src/util/viralloc.h | 145 +++-
2 files changed, 97 insertions(+), 249 deletions(-)



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [PATCH 1/7] util: purge all code for testing OOM handling

[Ján Tomko]

On Thu, Aug 29, 2019 at 07:02:44PM +0100, Daniel P. Berrangé wrote:

The OOM handling requires special build time options which we never
enable in our CI. Even once enabled the tests are incredibly slow and
typically require manual inspection of the results to weed out false
positives.

Since there was previous agreement to switch to abort on OOM in libvirt
code, there's no point continuing to keep the unused OOM testing code.

Signed-off-by: Daniel P. Berrangé 
---
configure.ac  |  17 ---
docs/docs.html.in |   3 -
docs/internals/oomtesting.html.in | 213 --
src/libvirt_private.syms  |   4 -
src/util/viralloc.c   | 111 
src/util/viralloc.h   |   5 -
tests/Makefile.am |   1 -
tests/oomtrace.pl |  41 --
tests/qemuxml2argvtest.c  |  12 +-
tests/testutils.c | 189 +-
tests/testutils.h |   2 -
tests/virfirewalltest.c   |  12 --
12 files changed, 6 insertions(+), 604 deletions(-)
delete mode 100644 docs/internals/oomtesting.html.in
delete mode 100755 tests/oomtrace.pl



Reviewed-by: Ján Tomko 

Jano


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

Re: [libvirt] [rust PATCH] Add list_all_volumes method for storage_pool::StoragePool

[Sahid Orentino Ferdjaoui]

On Thu, Aug 29, 2019 at 02:05:12AM -0700, Sage Imel wrote:
> From: Sage Imel 
> 
> Always returns the full list of volumes,
> can't just ask it how many volumes are in the pool
> 
> Signed-off-by: Sage Imel 
> ---

That looks to be a nice add, thank you for this contribution, can you
just consider the comments that i have added?

>  src/storage_pool.rs | 29 -
>  1 file changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/src/storage_pool.rs b/src/storage_pool.rs
> index 38676c2..e8ed21c 100644
> --- a/src/storage_pool.rs
> +++ b/src/storage_pool.rs
> @@ -18,7 +18,7 @@
>  
>  extern crate libc;
>  
> -use std::str;
> +use std::{str, ptr};
>  
>  use connect::sys::virConnectPtr;
>  use storage_vol::sys::virStorageVolPtr;
> @@ -57,6 +57,10 @@ extern "C" {
> xml: *const libc::c_char,
> flags: libc::c_uint)
> -> sys::virStoragePoolPtr;
> +fn virStoragePoolListAllVolumes(ptr: sys::virStoragePoolPtr,
> +vols: *mut *mut virStorageVolPtr,
> +flags:libc::c_uint)
> +-> libc::c_int;
>  fn virStoragePoolLookupByID(c: virConnectPtr, id: libc::c_int) -> 
> sys::virStoragePoolPtr;
>  fn virStoragePoolLookupByName(c: virConnectPtr,
>id: *const libc::c_char)
> @@ -103,6 +107,8 @@ pub const STORAGE_POOL_CREATE_WITH_BUILD: 
> StoragePoolCreateFlags = 1 << 0;
>  pub const STORAGE_POOL_CREATE_WITH_BUILD_OVERWRITE: StoragePoolCreateFlags = 
> 1 << 1;
>  pub const STORAGE_POOL_CREATE_WITH_BUILD_NO_OVERWRITE: 
> StoragePoolCreateFlags = 1 << 2;
>  
> +pub type VirStoragePoolListAllVolumesFlags = self::libc::c_uint;
> +

You well added the new type but as you can see in the other types we
prefer to remove the "vir" prefix. So that should be:

  pub type StoragePoolListAllVolumesFlags = self::libc::c_uint;

Also that, you have to declare the related flags, see:

  include/libvirt-storage.h

>  pub type StoragePoolState = self::libc::c_uint;
>  pub const VIR_STORAGE_POOL_INACTIVE: StoragePoolState = 0;
>  pub const VIR_STORAGE_POOL_BUILDING: StoragePoolState = 1;
> @@ -201,6 +207,27 @@ impl StoragePool {
>  }
>  }
>  
> +pub fn list_all_volumes(,
> +  flags: VirStoragePoolListAllVolumesFlags)
> +  -> Result, Error> {

Please consider to well align the code.

> +unsafe {

I know that you will find it everywhere but now we prefer to avoid
using a global "unsafe". It was not right. You should just use it when
it's necessary and also you should add a comment to explain why it is
safe. You can find an example in connect::open_auth().

> +let mut volumes: *mut virStorageVolPtr = ptr::null_mut();
> +let size =
> +virStoragePoolListAllVolumes(self.as_ptr(),  volumes, 
> flags as libc::c_uint);
> +if size == -1 {
> +return Err(Error::new());
> +}
> +
> +let mut array: Vec = Vec::new();
> +for x in 0..size as isize {
> +array.push(StorageVol::new(*volumes.offset(x)));
> +}
> +libc::free(volumes as *mut libc::c_void);
> +
> +return Ok(array);
> +}
> +}
> +
>  pub fn lookup_by_id(conn: , id: u32) -> Result Error> {
>  unsafe {
>  let ptr = virStoragePoolLookupByID(conn.as_ptr(), id as 
> libc::c_int);

It would be great if you can provide a test case or integration test
with it.

Thanks,
s.

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

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

[Kevin Wolf]

Am 23.08.2019 um 11:22 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 14.08.2019 13:07, Vladimir Sementsov-Ogievskiy wrote:
> > To get rid of implicit filters related workarounds in future let's
> > deprecate them now.
> 
> Interesting, could we deprecate implicit filter without deprecation of
> unnecessity of parameter? As actually, it's good when this parameter
> is not necessary, in most cases user is not interested in node-name.

A modern client is supposed to be interested in node-names. We basically
expect it to manage the whole block graph at a node level, so it should
certainly make sure to know the node-name of every node. It will need it
to take snapshots, insert filter drivers or anything like this on top of
the implicit node.

So once we make the option mandatory (which means returning an error for
legacy clients that don't do node-level management), I don't see a good
reason to ever make it optional again.

Kevin

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

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

[Kevin Wolf]

Am 30.08.2019 um 20:11 hat John Snow geschrieben:
> 
> 
> On 8/30/19 6:07 AM, Christophe de Dinechin wrote:
> > Without having looked at the code much, I think I would
> > 
> > 1. extend the existing QAPI error to support warnings, deprecations and
> >info messages. The first problem I see is that there is no error, so
> >we may sometimes need to create one when there was none before. And
> >of course make sure that this does not ultimately show as an error,
> >but as a success with additional annotations.
> > 
> 
> I assume this might be a chance to consolidate all of the methodologies
> we use for actually checking if there was an error or not. There have
> been many and I am sure Markus can give us a history lesson if it's
> warranted.
> 
> Generally, there's a few paradigms I see a lot:
> 
> 1. Rely on an error return code being produced by the called function.
> The caller trusts that errp was set. This is one of my favorite methods,
> because it has the least scaffolding.

This one is convenient to use, but the problem is that nobody enforces
that errp is always set if ret < 0, and that it's not set for ret == 0.
So in a way it is error-prone because it allows inconsistencies.

> 2. Pass errp directly to the called function, and check for null after
> return. I don't like this method very much, because of confusion with:

I mainly don't like this very much because it's simply wrong.

Callers can pass errp = NULL if they aren't interested in error
information. If you directly pass errp, you can't check *errp because
errp could be NULL.

So directly passing errp makes the code simpler, but only use it in
functions where you don't intend to check whether an error is set.

> 3. Create a local error object; check THAT for null, and propagate the
> error to the common error object. I think Markus has explained why we
> have this code 50 times, and I forget again minutes later.

local_err exists for those cases where you need to check the error
object before passing it to the caller. (And obviously for those cases
where you don't want to pass it to the caller, but do something like
error_report_err().)

> If we want to expand the concept of the error object into something that
> encompasses hints, warnings and deprecations*, checking for null is no
> longer appropriate. It might be a good chance to make our error
> propagation story more consistent, too.
> 
> We could unify with a helper like this, I think, if I'm not forgetting
> some crucial usage detail:
> 
> subroutine(foo, bar, errp);
> if (failure(errp)) {
> error_append_hint(errp, "Lorem ipsum, ...");
> cleanup();
> return;
> }
> 
> We would then always use this pattern that operates directly on the
> caller's errp instead of creating local error objects to allow hints and
> warnings to accumulate.

There are two parts to the change that you imply:

1. Forbid passing errp = NULL to any function so that *errp can always
   be checked. This gets rid of local_err in the intermediate function,
   but may require the introduction of new local_err variables in
   top-level callers which ignore the error information.

2. Introduce failure(errp) to replace errp != NULL because we want Error
   to contain warnings and notes, too. Currently, it can contain only
   exactly one error, so this would be a major change.

   Note that the change wouldn't make the existing 'if (errp)' checks
   build failures, so getting confident that we found and replaced all
   of them is going to be hard.

Essentially, you'd probably want to replace Error with a new type so
that the compiler will at least be able to tell which places have been
converted and which haven't.

And then, you'd have to touch every single function that does something
with errors. This is a huge change across the whole source tree.

I doubt it's worth the effort.

> > Second, why not report the use of deprecated features? I don't fully buy
> > the rationale that libvirt engages the features, because it does not do
> > it on its own, it does it because the user made some specific request.
> 
> Because the user didn't request those specific QMP features, they asked
> for the VM to start, or to stop, or they asked for a backup, or a snapshot.
> 
> On my desktop, I am not really too interested in knowing if XFCE is
> using deprecated features of xorg or wayland. I didn't tell it to use
> them and I have no real power or control over that. It's nice if I'm a
> developer, but as a user, it's noise.
> 
> So a development log seems right for these, but not user-visible
> interruptions.

I agree, it's not the high-level operation the user requested that is
deprecated, but just the specific implementation libvirt uses to perform
the operation on a QEMU VM.

The expected response to a deprecation notice is that a libvirt update
makes it go away by using more modern interfaces, not that the user
changes their workflow.

Kevin

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

Re: [libvirt] [glib PATCH 0/2] Add gvir_config_domain_os_get_firmware() + _domain_os_get_machine() test

[Michal Privoznik]

On 8/26/19 7:03 PM, Fabiano Fidêncio wrote:

This series adds a way to retrive the DomainOs' Firmware set (with
tests) and, while touching that code, also adds one more check for
gvir_config_domain_os_get_machine().

Fabiano Fidêncio (2):
   gconfig: Add _domain_os_get_firmware()
   tests,test-gconfig: Check _domain_os_get_machine()

  libvirt-gconfig/libvirt-gconfig-domain-os.c | 12 
  libvirt-gconfig/libvirt-gconfig-domain-os.h |  1 +
  libvirt-gconfig/libvirt-gconfig.sym |  1 +
  tests/test-gconfig.c|  3 +++
  4 files changed, 17 insertions(+)



Reviewed-by: Michal Privoznik 

Michal

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

Re: [libvirt] [glib PATCH 0/2] Add gvir_config_domain_os_get_firmware() + _domain_os_get_machine() test

[Fabiano Fidêncio]

On Mon, Aug 26, 2019 at 7:03 PM Fabiano Fidêncio  wrote:
>
> This series adds a way to retrive the DomainOs' Firmware set (with
> tests) and, while touching that code, also adds one more check for
> gvir_config_domain_os_get_machine().
>
> Fabiano Fidêncio (2):
>   gconfig: Add _domain_os_get_firmware()
>   tests,test-gconfig: Check _domain_os_get_machine()
>
>  libvirt-gconfig/libvirt-gconfig-domain-os.c | 12 
>  libvirt-gconfig/libvirt-gconfig-domain-os.h |  1 +
>  libvirt-gconfig/libvirt-gconfig.sym |  1 +
>  tests/test-gconfig.c|  3 +++
>  4 files changed, 17 insertions(+)
>
> --
> 2.21.0
>

ping!

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

Re: [libvirt] Availability of libvirt 5.7.0 Release Candidate 2

[Andrea Bolognani]

On Sun, 2019-09-01 at 21:13 +0200, Daniel Veillard wrote:
>   I have just tagged RC2 in git and pushed the signed tarball and source rpm 
> to
> the usual place:
> https://libvirt.org/sources/
> 
>  Seems fine in my minimal testing, there is two red spots in CI at
> CI https://ci.centos.org/view/libvirt/ but on different components than the 
> ones
> for RC1 so I would assume some fuzzyness in Jenkins, not libvirt itself.

The libosinfo issue looks entirely unrelated to libvirt, so we should
be safe on that side.

Not sure what to make of the virt-manager issue[1], as I'm not familiar
enough with the project. Cole, Pavel?


[1] 
https://ci.centos.org/view/libvirt/job/virt-manager-check/1054/systems=libvirt-fedora-rawhide/console
-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] Availability of libvirt 5.7.0 Release Candidate 2

[Michal Privoznik]

On 9/1/19 9:13 PM, Daniel Veillard wrote:


   I have just tagged RC2 in git and pushed the signed tarball and source rpm to
the usual place:
 https://libvirt.org/sources/


Sorry for raising this issue this late, but I found out only during 
weekend when I tried to build libvirt on my home machine. The build with 
--with-xenapi is broken:


  CC   xenapi/libvirt_driver_xenapi_la-xenapi_driver.lo
xenapi/xenapi_driver.c:38:10: fatal error: xen_common.h: No such file or 
directory

 #include "xen_common.h"
  ^~
compilation terminated.


The commit that introduced this problem is ecc4d75d01 and reverting it 
solves the issue.


Michal

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

Re: [libvirt] Error when creating VM with persistent memory

[Michal Privoznik]

On 8/31/19 7:33 PM, Seema Pandit wrote:

Hi Michal,
Thank you for the reply.
I was having issues compiling qemu code on fedora29. So instead of dropping
prealloc in virsh, tried adding prealloc=yes in qemu command line.
prealloc=yes works. It does not lead to using more system memory when using
DAX.
+Dan
Here are the steps:

ndctl create-namespace -t pmem -m fsdax --align=4k -s 400G

mkfs.ext4 /dev/pmem0

mount -o dax /dev/pmem0 /mnt/pmem0

dd if=/dev/zero of=/mnt/pmem0/file1 bs=4k count=104857600

[root@system-name]# dd if=/dev/zero of=/mnt/pmem0/file1 bs=4k
count=104857600

dd: error writing '/mnt/pmem0/file1': No space left on device

101313980+0 records in

101313979+0 records out

414982057984 bytes (415 GB, 386 GiB) copied, 946.495 s, 438 MB/s


Slightly smaller file is created than asked.

[root@system-name]# du -sh

387G.


sample qemu command line which works:

qemu-system-x86_64 \

-name test \

-drive
file=/var/lib/libvirt/images/test-ad.qcow2,format=qcow2,index=0,media=disk
\ -m 2G,slots=4,maxmem=428G \ -smp 2 \ -machine pc,accel=kvm,nvdimm=on \
-enable-kvm \ -object
memory-backend-file,id=pmem1,prealloc=yes,share=on,mem-path=/mnt/pmem0/file1,size=386G,align=4K
\ -device nvdimm,memdev=pmem1,id=nv1 \ -daemonize


Looks like the only difference to the cmd line generated by libvirt and 
this one then is align=4K. To confirm that, can you share the full qemu 
cmd line as generated by libvirt please?
Libvirt does not do anything special with guest memory, so this is 
matter of qemu cmd line and that's why we need to see what's different, 
what works and what doesn't. Then we have some lead to understand the 
problem IMO.


Michal

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