Re: [libvirt] [PATCH V3 jenkins-ci 0/2] Add support for openSUSE

[Jim Fehlig]

On 12/6/19 4:44 PM, Jim Fehlig wrote:
> Jim Fehlig (2):
>guests: Add support for openSUSE
>guests: Add lci build support for openSUSE

I forgot to mention, 'lcitool update libvirt-opensuse-151 libvirt' still fails 
in the Configure hostname task

TASK [Configure hostname] 
**
fatal: [libvirt-opensuse-151]: FAILED! => {"changed": false, "msg": "hostname 
module cannot be used on platform Linux (Opensuse-leap)"}

> 
>   guests/configs/autoinst.xml   | 77 +++
>   .../host_vars/libvirt-opensuse-151/docker.yml |  2 +
>   .../libvirt-opensuse-151/install.yml  |  2 +
>   .../host_vars/libvirt-opensuse-151/main.yml   | 10 +++
>   guests/inventory  |  1 +
>   guests/lcitool|  2 +
>   guests/playbooks/build/jobs/defaults.yml  |  1 +
>   guests/playbooks/build/projects/libvirt.yml   |  1 +
>   guests/playbooks/update/tasks/base.yml| 15 
>   guests/vars/mappings.yml  | 38 -
>   10 files changed, 145 insertions(+), 4 deletions(-)
>   create mode 100644 guests/configs/autoinst.xml
>   create mode 100644 guests/host_vars/libvirt-opensuse-151/docker.yml
>   create mode 100644 guests/host_vars/libvirt-opensuse-151/install.yml
>   create mode 100644 guests/host_vars/libvirt-opensuse-151/main.yml
> 


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

[libvirt] [PATCH V3 jenkins-ci 1/2] guests: Add support for openSUSE

[Jim Fehlig]

This change adds support for installing and updating openSUSE Leap 15.1
using lcilool.

Signed-off-by: Jim Fehlig 
---
 guests/configs/autoinst.xml   | 77 +++
 .../host_vars/libvirt-opensuse-151/docker.yml |  2 +
 .../libvirt-opensuse-151/install.yml  |  2 +
 .../host_vars/libvirt-opensuse-151/main.yml   | 10 +++
 guests/inventory  |  1 +
 guests/lcitool|  2 +
 guests/playbooks/update/tasks/base.yml| 15 
 guests/vars/mappings.yml  | 38 -
 8 files changed, 143 insertions(+), 4 deletions(-)

diff --git a/guests/configs/autoinst.xml b/guests/configs/autoinst.xml
new file mode 100644
index 000..f8ec3df
--- /dev/null
+++ b/guests/configs/autoinst.xml
@@ -0,0 +1,77 @@
+
+
+http://www.suse.com/1.0/yast2ns";
+  xmlns:config="http://www.suse.com/1.0/configns";>
+  
+
+  false
+  false
+
+  
+  
+
+  /dev/vda
+  all
+  
+
+  swap
+  256M
+  swap
+
+
+  ext4
+  /
+  max
+
+  
+
+  
+  
+
+  console serial
+
+  
+  
+UTC
+UTC
+  
+  
+false
+
+  openSUSE
+
+
+  base
+  minimal_base
+  yast2_basis
+
+
+  openssh
+  hostname
+
+  
+  
+true
+  
+  
+
+  root
+  root
+  false
+  0
+  0
+  /root
+  /bin/bash
+
+  
+  
+multi-user
+
+  
+sshd
+enable
+  
+
+  
+
diff --git a/guests/host_vars/libvirt-opensuse-151/docker.yml 
b/guests/host_vars/libvirt-opensuse-151/docker.yml
new file mode 100644
index 000..6ba7adb
--- /dev/null
+++ b/guests/host_vars/libvirt-opensuse-151/docker.yml
@@ -0,0 +1,2 @@
+---
+docker_base: opensuse/leap:15.1
diff --git a/guests/host_vars/libvirt-opensuse-151/install.yml 
b/guests/host_vars/libvirt-opensuse-151/install.yml
new file mode 100644
index 000..d0fdbe5
--- /dev/null
+++ b/guests/host_vars/libvirt-opensuse-151/install.yml
@@ -0,0 +1,2 @@
+---
+install_url: http://download.opensuse.org/distribution/leap/15.1/repo/oss/
diff --git a/guests/host_vars/libvirt-opensuse-151/main.yml 
b/guests/host_vars/libvirt-opensuse-151/main.yml
new file mode 100644
index 000..f422a9e
--- /dev/null
+++ b/guests/host_vars/libvirt-opensuse-151/main.yml
@@ -0,0 +1,10 @@
+---
+projects:
+  - libvirt
+
+package_format: 'rpm'
+package_manager: 'zypper'
+os_name: 'OpenSUSE'
+os_version: '151'
+
+ansible_python_interpreter: /usr/bin/python3
diff --git a/guests/inventory b/guests/inventory
index 3b15513..f1f7708 100644
--- a/guests/inventory
+++ b/guests/inventory
@@ -8,5 +8,6 @@ libvirt-fedora-rawhide
 libvirt-freebsd-11
 libvirt-freebsd-12
 libvirt-freebsd-current
+libvirt-opensuse-151
 libvirt-ubuntu-16
 libvirt-ubuntu-18
diff --git a/guests/lcitool b/guests/lcitool
index d617beb..8436ce7 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -531,6 +531,8 @@ class Application:
 install_config = "preseed.cfg"
 elif facts["os_name"] in ["CentOS", "Fedora"]:
 install_config = "kickstart.cfg"
+elif facts["os_name"] == "OpenSUSE":
+install_config = "autoinst.xml"
 else:
 raise Exception(
 "Host {} doesn't support installation".format(host)
diff --git a/guests/playbooks/update/tasks/base.yml 
b/guests/playbooks/update/tasks/base.yml
index 3d83e78..be4ae21 100644
--- a/guests/playbooks/update/tasks/base.yml
+++ b/guests/playbooks/update/tasks/base.yml
@@ -65,12 +65,27 @@
   when:
 - package_format == 'pkg'
 
+- name: Update installed packages
+  command: '{{ package_manager }} update -y -l --force-resolution 
--no-recommends'
+  args:
+warn: no
+  when:
+- os_name == 'OpenSUSE'
+
 - name: Clean up packages after update
   shell: '{{ package_manager }} clean packages -y && {{ package_manager }} 
autoremove -y'
   args:
 warn: no
   when:
 - package_format == 'rpm'
+- os_name != "OpenSUSE"
+
+- name: Clean up packages after update
+  shell: '{{ package_manager }} clean'
+  args:
+warn: no
+  when:
+- os_name == "OpenSUSE"
 
 - name: Clean up packages after update
   apt:
diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml
index f5dab6a..fce2028 100644
--- a/guests/vars/mappings.yml
+++ b/guests/vars/mappings.yml
@@ -19,10 +19,10 @@
 #   - deb, pkg, rpm
 #
 # Valid OS names are:
-#   - CentOS, Debian, Fedora, FreeBSD, Ubuntu
+#   - CentOS, Debian, Fedora, FreeBSD, OpenSUSE, Ubuntu
 #
 # Valid OS versions are:
-#   - CentOS7, Debian9, FedoraRawhide, Ubuntu18 and so on
+#   - CentOS7, Debian9, FedoraRawhide, OpenSUSE151, Ubuntu18 and so on
 #
 # The arch specific rules use a prefix "$ARCH-" where  $ARCH
 # is a libvirt arch name.
@@ -70,6 +70,7 @@ mappings:
 
   apparmor:
 deb: libapparmor-dev
+OpenSUSE: libapparmor-devel
 cross-policy-deb:

[libvirt] [PATCH V3 jenkins-ci 0/2] Add support for openSUSE

[Jim Fehlig]

Jim Fehlig (2):
  guests: Add support for openSUSE
  guests: Add lci build support for openSUSE

 guests/configs/autoinst.xml   | 77 +++
 .../host_vars/libvirt-opensuse-151/docker.yml |  2 +
 .../libvirt-opensuse-151/install.yml  |  2 +
 .../host_vars/libvirt-opensuse-151/main.yml   | 10 +++
 guests/inventory  |  1 +
 guests/lcitool|  2 +
 guests/playbooks/build/jobs/defaults.yml  |  1 +
 guests/playbooks/build/projects/libvirt.yml   |  1 +
 guests/playbooks/update/tasks/base.yml| 15 
 guests/vars/mappings.yml  | 38 -
 10 files changed, 145 insertions(+), 4 deletions(-)
 create mode 100644 guests/configs/autoinst.xml
 create mode 100644 guests/host_vars/libvirt-opensuse-151/docker.yml
 create mode 100644 guests/host_vars/libvirt-opensuse-151/install.yml
 create mode 100644 guests/host_vars/libvirt-opensuse-151/main.yml

-- 
2.24.0


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

[libvirt] [PATCH V3 jenkins-ci 2/2] guests: Add lci build support for openSUSE

[Jim Fehlig]

Signed-off-by: Jim Fehlig 
---
 guests/playbooks/build/jobs/defaults.yml| 1 +
 guests/playbooks/build/projects/libvirt.yml | 1 +
 2 files changed, 2 insertions(+)

diff --git a/guests/playbooks/build/jobs/defaults.yml 
b/guests/playbooks/build/jobs/defaults.yml
index 48cf643..43ab882 100644
--- a/guests/playbooks/build/jobs/defaults.yml
+++ b/guests/playbooks/build/jobs/defaults.yml
@@ -10,6 +10,7 @@ all_machines:
   - libvirt-freebsd-11
   - libvirt-freebsd-12
   - libvirt-freebsd-current
+  - libvirt-opensuse-151
   - libvirt-ubuntu-16
   - libvirt-ubuntu-18
 rpm_machines:
diff --git a/guests/playbooks/build/projects/libvirt.yml 
b/guests/playbooks/build/projects/libvirt.yml
index 4b3dfdf..66ea851 100644
--- a/guests/playbooks/build/projects/libvirt.yml
+++ b/guests/playbooks/build/projects/libvirt.yml
@@ -19,6 +19,7 @@
   - libvirt-fedora-30
   - libvirt-fedora-31
   - libvirt-fedora-rawhide
+  - libvirt-opensuse-151
   - libvirt-ubuntu-16
   - libvirt-ubuntu-18
 - include: '{{ playbook_base }}/jobs/autotools-check-job.yml'
-- 
2.24.0


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

Re: [libvirt] [RFC PATCH 1/9] vfio/pci: introduce mediate ops to intercept vfio-pci ops

[Eric Blake]

On 12/4/19 9:25 PM, Yan Zhao wrote:

when vfio-pci is bound to a physical device, almost all the hardware
resources are passthroughed.


The intent is obvious, but it sounds awkward to a native speaker.
s/passthroughed/passed through/


Sometimes, vendor driver of this physcial device may want to mediate some


physical


hardware resource access for a short period of time, e.g. dirty page
tracking during live migration.

Here we introduce mediate ops in vfio-pci for this purpose.

Vendor driver can register a mediate ops to vfio-pci.
But rather than directly bind to the passthroughed device, the


passed-through

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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

Re: [libvirt] [PATCH 8/8] qemu: migration: Properly setup mirror for blockdev configurations

[Eric Blake]

On 12/6/19 12:09 PM, Peter Krempa wrote:

With blockdev we need to refer to the nodename of the disk source image
as the source argument for the blockdev-mirror operation while still
keeping the old job name. With blockdev we must also persist the job in
qemu.

Signed-off-by: Peter Krempa 
---
  src/qemu/qemu_migration.c | 24 
  1 file changed, 20 insertions(+), 4 deletions(-)



Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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

Re: [libvirt] [PATCH 7/8] qemu: migration: Mention disk target rather than the drive name in debug msg

[Eric Blake]

On 12/6/19 12:09 PM, Peter Krempa wrote:

Signed-off-by: Peter Krempa 
---
  src/qemu/qemu_migration.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)



Reviewed-by: Eric Blake 


diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 8bf23e9f30..27023c94b1 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -829,7 +829,7 @@ qemuMigrationSrcNBDStorageCopyBlockdev(virQEMUDriverPtr 
driver,
  int mon_ret = 0;
  g_autoptr(virStorageSource) copysrc = NULL;

-VIR_DEBUG("starting blockdev mirror for disk=%s to host=%s", diskAlias, 
host);
+VIR_DEBUG("starting blockdev mirror for disk=%s to host=%s", disk->dst, 
host);

  if (!(copysrc = qemuMigrationSrcNBDStorageCopyBlockdevPrepareSource(disk, 
host, port, tlsAlias)))
  return -1;



--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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

Re: [libvirt] [PATCH 6/8] qemu: migration: Split out setup of the migration target

[Eric Blake]

On 12/6/19 12:09 PM, Peter Krempa wrote:

Separate out allocation of the virStorageSource corresponding to the
target NBD export of the migration.

As part of the splitout we allocate the export name explicitly as that
one must not change regardless whether blockdev is used or not to
provide compatibility.

Signed-off-by: Peter Krempa 
---
  src/qemu/qemu_migration.c | 55 +--
  1 file changed, 35 insertions(+), 20 deletions(-)


Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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

Re: [libvirt] [PATCH 5/8] qemu: blockjob: Allow NULL 'mirror' for block copy jobs due to migration

[Eric Blake]

On 12/6/19 12:09 PM, Peter Krempa wrote:

The non-shared-storage migration tracks the storage source used
explicitly in the migration data so we must allow for processing of the
block job which has NULL mirror as the mirror will not be populated.

Signed-off-by: Peter Krempa 
---
  src/qemu/qemu_blockjob.c | 11 ---
  1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index baa79ea80c..2773acc990 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -1139,7 +1139,9 @@ 
qemuBlockJobProcessEventConcludedCopyPivot(virQEMUDriverPtr driver,
  {
  VIR_DEBUG("copy job '%s' on VM '%s' pivoted", job->name, vm->def->name);

-if (!job->disk)
+/* mirror may be NULL for copy job corresponding to migragion */


migration (3 times)

With typos fixed,
Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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

Re: [libvirt] [PATCH 4/8] qemu: migration: Simplify cleanup in qemuMigrationSrcNBDCopyCancelOne

[Eric Blake]

On 12/6/19 12:09 PM, Peter Krempa wrote:

Now that the cleanup section does not exist remove the label.

Signed-off-by: Peter Krempa 
---
  src/qemu/qemu_migration.c | 15 +--
  1 file changed, 5 insertions(+), 10 deletions(-)


Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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

Re: [libvirt] [PATCH 3/8] qemu: migration: Access job name from job struct

[Eric Blake]

On 12/6/19 12:09 PM, Peter Krempa wrote:

qemuMigrationSrcNBDCopyCancelOne uses the block job data structure but
generated it's own job name rather than taking it from the block job
data.

Signed-off-by: Peter Krempa 
---
  src/qemu/qemu_migration.c | 7 +--
  1 file changed, 1 insertion(+), 6 deletions(-)



Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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

Re: [libvirt] [RFC PATCH 1/9] vfio/pci: introduce mediate ops to intercept vfio-pci ops

[Alex Williamson]

On Fri, 6 Dec 2019 02:56:55 -0500
Yan Zhao  wrote:

> On Fri, Dec 06, 2019 at 07:55:19AM +0800, Alex Williamson wrote:
> > On Wed,  4 Dec 2019 22:25:36 -0500
> > Yan Zhao  wrote:
> >   
> > > when vfio-pci is bound to a physical device, almost all the hardware
> > > resources are passthroughed.
> > > Sometimes, vendor driver of this physcial device may want to mediate some
> > > hardware resource access for a short period of time, e.g. dirty page
> > > tracking during live migration.
> > > 
> > > Here we introduce mediate ops in vfio-pci for this purpose.
> > > 
> > > Vendor driver can register a mediate ops to vfio-pci.
> > > But rather than directly bind to the passthroughed device, the
> > > vendor driver is now either a module that does not bind to any device or
> > > a module binds to other device.
> > > E.g. when passing through a VF device that is bound to vfio-pci modules,
> > > PF driver that binds to PF device can register to vfio-pci to mediate
> > > VF's regions, hence supporting VF live migration.
> > > 
> > > The sequence goes like this:
> > > 1. Vendor driver register its vfio_pci_mediate_ops to vfio-pci driver
> > > 
> > > 2. vfio-pci maintains a list of those registered vfio_pci_mediate_ops
> > > 
> > > 3. Whenever vfio-pci opens a device, it searches the list and call
> > > vfio_pci_mediate_ops->open() to check whether a vendor driver supports
> > > mediating this device.
> > > Upon a success return value of from vfio_pci_mediate_ops->open(),
> > > vfio-pci will stop list searching and store a mediate handle to
> > > represent this open into vendor driver.
> > > (so if multiple vendor drivers support mediating a device through
> > > vfio_pci_mediate_ops, only one will win, depending on their registering
> > > sequence)
> > > 
> > > 4. Whenever a VFIO_DEVICE_GET_REGION_INFO ioctl is received in vfio-pci
> > > ops, it will chain into vfio_pci_mediate_ops->get_region_info(), so that
> > > vendor driver is able to override a region's default flags and caps,
> > > e.g. adding a sparse mmap cap to passthrough only sub-regions of a whole
> > > region.
> > > 
> > > 5. vfio_pci_rw()/vfio_pci_mmap() first calls into
> > > vfio_pci_mediate_ops->rw()/vfio_pci_mediate_ops->mmaps().
> > > if pt=true is rteturned, vfio_pci_rw()/vfio_pci_mmap() will further
> > > passthrough this read/write/mmap to physical device, otherwise it just
> > > returns without touch physical device.
> > > 
> > > 6. When vfio-pci closes a device, vfio_pci_release() chains into
> > > vfio_pci_mediate_ops->release() to close the reference in vendor driver.
> > > 
> > > 7. Vendor driver unregister its vfio_pci_mediate_ops when driver exits
> > > 
> > > Cc: Kevin Tian 
> > > 
> > > Signed-off-by: Yan Zhao 
> > > ---
> > >  drivers/vfio/pci/vfio_pci.c | 146 
> > >  drivers/vfio/pci/vfio_pci_private.h |   2 +
> > >  include/linux/vfio.h|  16 +++
> > >  3 files changed, 164 insertions(+)
> > > 
> > > diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> > > index 02206162eaa9..55080ff29495 100644
> > > --- a/drivers/vfio/pci/vfio_pci.c
> > > +++ b/drivers/vfio/pci/vfio_pci.c
> > > @@ -54,6 +54,14 @@ module_param(disable_idle_d3, bool, S_IRUGO | S_IWUSR);
> > >  MODULE_PARM_DESC(disable_idle_d3,
> > >"Disable using the PCI D3 low power state for idle, unused 
> > > devices");
> > >  
> > > +static LIST_HEAD(mediate_ops_list);
> > > +static DEFINE_MUTEX(mediate_ops_list_lock);
> > > +struct vfio_pci_mediate_ops_list_entry {
> > > + struct vfio_pci_mediate_ops *ops;
> > > + int refcnt;
> > > + struct list_headnext;
> > > +};
> > > +
> > >  static inline bool vfio_vga_disabled(void)
> > >  {
> > >  #ifdef CONFIG_VFIO_PCI_VGA
> > > @@ -472,6 +480,10 @@ static void vfio_pci_release(void *device_data)
> > >   if (!(--vdev->refcnt)) {
> > >   vfio_spapr_pci_eeh_release(vdev->pdev);
> > >   vfio_pci_disable(vdev);
> > > + if (vdev->mediate_ops && vdev->mediate_ops->release) {
> > > + vdev->mediate_ops->release(vdev->mediate_handle);
> > > + vdev->mediate_ops = NULL;
> > > + }
> > >   }
> > >  
> > >   mutex_unlock(&vdev->reflck->lock);
> > > @@ -483,6 +495,7 @@ static int vfio_pci_open(void *device_data)
> > >  {
> > >   struct vfio_pci_device *vdev = device_data;
> > >   int ret = 0;
> > > + struct vfio_pci_mediate_ops_list_entry *mentry;
> > >  
> > >   if (!try_module_get(THIS_MODULE))
> > >   return -ENODEV;
> > > @@ -495,6 +508,30 @@ static int vfio_pci_open(void *device_data)
> > >   goto error;
> > >  
> > >   vfio_spapr_pci_eeh_open(vdev->pdev);
> > > + mutex_lock(&mediate_ops_list_lock);
> > > + list_for_each_entry(mentry, &mediate_ops_list, next) {
> > > + u64 caps;
> > > + u32 handle;  
> > 
> > Wouldn't it seem likely that the ops provider might use this handle 

Re: [libvirt] [PATCH 2/8] qemu: migration: Properly export backend for NBD storage migration

[Eric Blake]

On 12/6/19 12:09 PM, Peter Krempa wrote:

With -blockdev we must use the nodename as the export but we must keep
the name of the export as it was before to ensure compatiblity.


compatibility

Makes sense - this is code on the destination, but we may be migrating 
storage from an older libvirt that wasn't using blockdev, so we must 
export the same name as expected by that older client (our goal is to 
allow migration as a way to upgrade from a -device command line to a 
-blockdev command line, without any guest downtime)


Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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

Re: [libvirt] [PATCH 1/8] qemu: migration: Simplify handling of 'diskAlias' when adding NBD exports

[Eric Blake]

On 12/6/19 12:09 PM, Peter Krempa wrote:

Declare the variable inside the loop with automatic clearing.

Signed-off-by: Peter Krempa 
---
  src/qemu/qemu_migration.c | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)


Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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

Re: [libvirt] [PATCH for-4.2?] block/qcow2-bitmap: fix crash bug in qcow2_co_remove_persistent_dirty_bitmap

[Eric Blake]

On 12/6/19 1:02 PM, John Snow wrote:


I'm afraid that the only thing is not remove persistent bitmaps, which
were never synced to the image. So, instead the sequence above, we need


1. create persistent bitmap A
2. shutdown vm
3. start vm
4. create persistent bitmap B
5. remember, that we want to remove bitmap B after vm shutdown
...
    some other operations
...
6. vm shutdown
7. start vm in stopped mode, and remove all bitmaps marked for removing
8. stop vm

But, I think that in real circumstances, vm shutdown is rare thing...


Part of me wonders if we would have detected this MUCH sooner if I had 
gotten my wish of having the qcow2 metadata updated on creation of any 
persistent bitmap (not actually writing out the bitmap itself, just 
updating the bitmap table to mark that there is a new persistent 
inconsistent bitmap), so that a) qemu-img info -U can see the persistent 
bitmap's existence, and b) an unexpected abrupt crash of qemu does not 
lose the existence of the bitmap.  At the time I raised the question, 
the push-back at the time was a desire to minimize writes to the qcow2 
metadata at all costs, warranting our current extreme code contortions 
to keep persistent bitmaps were kept in memory only until VM shutdown. 
But had we been doing it, we would spot problems like this without 
having to do VM shutdown, and our code might actually be simpler than 
our current contortions.  Maybe we should still revisit that decision 
(of course, that question is independent of this patch, and therefore 
5.0 material at earliest).


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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

Re: [libvirt] Offline manipulation of Dirty Bitmaps by qemu-img

[John Snow]

On 12/6/19 5:37 AM, Vladimir Sementsov-Ogievskiy wrote:
> 06.12.2019 1:37, John Snow wrote:
>> This has come up in the past, and I believe we discussed this at KVM
>> Forum, too:
>>
>> There have been requests from oVirt (via Nir Soffer) to add some offline
>> bitmap manipulation functionality. In the past, our stance has generally
>> been "Use QEMU without an accelerator, and use QMP to manipulate the
>> images."
>>
>> We like this for a few reasons:
>>
>> 1. It keeps bitmap management code tightly centralized
>> 2. It allows for the full suite of bitmap manipulations in either
>> offline or online mode with one tool
>> 3. We wouldn't have to write new code.
>> 4. Or design new CLIs and duplicate our existing work.
>> 5. Or write even more tests.
>>
>> However, tools like oVirt may or may not be fully equipped to launch
>> QEMU in this context, and there is always a desire for qemu-img to be
>> able to "do more", so existing management suites could extend
>> functionality more easily.
> 
> I think, all guys, who don't want to use Qemu directly for image 
> manipulations,
> should hope for Kevin's "[RFC PATCH 00/18] Add qemu-storage-daemon", which is
> the correct solution of their problem. Still, I'm not one of these guys.
> 
>>
>> (Or so I am imagining.)
>>
>> I am still leaning heavily against adding any more CLI commands or
>> options to qemu-img right now. Even if we do add some of the fundamental
>> ones like "add" or "remove", it seems only a matter of time before we
>> have to add "clear", "merge", etc. Is this just a race to code duplication?
>>
>> On the other hand, one of the other suggestions is to have qemu-img
>> check --repair optionally delete corrupted bitmaps. I kind of like this
>> idea: it's a buyer beware operation that might make management layers
>> unhappy, but then again ... repair is always something that could make
>> things worse.
>>
>> Plus, if you manage to corrupt bitmaps badly enough that they can't even
>> be parsed, you might need a heavyweight repair operation.
>>
> 
> Improving "check" is a correct thing, because it's done inside qcow2 driver
> itself. We just don't have corresponding qmp command or command line option
> for Qemu to use this thing (or I missed it).
> 

OK, I agree. I made a redhat BZ to track that we want this: 1780416 -
RFE: qemu-img check --repair should optionally remove any
corrupted bitmaps

I'll work on a patch and we can debate the details there.

--js

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

Re: [libvirt] [PATCH for-4.2?] block/qcow2-bitmap: fix crash bug in qcow2_co_remove_persistent_dirty_bitmap

[John Snow]

On 12/6/19 9:36 AM, Eric Blake wrote:
> [adding in Peter Maydell, as there is now potential talk of other
> 4.2-worthy patches]
> 
> On 12/6/19 4:18 AM, Vladimir Sementsov-Ogievskiy wrote:
>> 05.12.2019 23:16, John Snow wrote:
>>>
>>>
>>> On 12/5/19 3:09 PM, Eric Blake wrote:
 On 12/5/19 1:30 PM, Vladimir Sementsov-Ogievskiy wrote:
> Here is double bug:
>
> First, return error but not set errp. This may lead to:
> qmp block-dirty-bitmap-remove may report success when actually failed
>
> block-dirty-bitmap-remove used in a transaction will crash, as
> qmp_transaction will think that it returned success and will cal

 call

> block_dirty_bitmap_remove_commit which will crash, as state->bitmap is
> NULL
>
> Second (like in anecdote), this case is not an error at all. As it is
> documented in the comment above bdrv_co_remove_persistent_dirty_bitmap
> definition, absence of bitmap is not an error, and similar case
> handled
> at start of qcow2_co_remove_persistent_dirty_bitmap, it returns 0 when
> there is no bitmaps at all..

 double .

>
> But when there are some bitmaps, but not the requested one, it return
> error with errp unset.
>
> Fix that.
>
> Fixes: b56a1e31759b750
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>
> Hi all!
>
> Ohm, suddenly we faced this bug. It's a regression in 4.2. I'm very
> sorry for introducing it, and it sad that it's found so late..
>
> Personally, I think that this one worth rc5, as it makes new bitmap
> interfaces unusable. But the decision is yours.
>
> Last minute edit: hmm, actually, transaction action introduced in
> 4.2, so crash is not a regression, only broken
> block-dirty-bitmap-remove
> command is a regression... Maybe it's OK for stable.

 Libvirt REALLY wants to use transaction bitmap management (and require
 qemu 4.2) for its incremental backup patches that Peter is almost ready
 to merge in.  I'm trying to ascertain:

 When exactly can this bug hit?  Can you give a sequence of QMP commands
 that will trigger it for easy reproduction?  Are there workarounds
 (such
 as checking that a bitmap exists prior to attempting to remove it)?  If
 it does NOT get fixed in rc5, how will libvirt be able to probe whether
 the fix is in place?

>>>
>>> It looks like:
>>>
>>> - You need to have at least one bitmap
>>> - You need to use transactional remove
>>> - you need to misspell the bitmap name
>>> - The remove will fail (return -EINVAL) but doesn't set errp
>>> - The caller chokes on incomplete information, state->bitmap is NULL
>>
>>
>> No, that would be too simple, the thing is worse. Absolutele correct
>> removing is broken, without any misspelling
>>
>> Bug triggers when we are removing persistent bitmap that is not stored
>> yet in the image AND at least one (another) bitmap already stored in
>> the image. So, something like:
>>
>> 1. create persistent bitmap A
>> 2. shutdown vm  (bitmap A is synced)
>> 3. start vm
>> 4. create persistent bitmap B
>> 5. remove bitmap B - it fails (and crashes if in transaction)
>>
>> 
>>
>> Hmm, workaround..
>>
>> I'm afraid that the only thing is not remove persistent bitmaps, which
>> were never synced to the image. So, instead the sequence above, we need
>>
>>
>> 1. create persistent bitmap A
>> 2. shutdown vm
>> 3. start vm
>> 4. create persistent bitmap B
>> 5. remember, that we want to remove bitmap B after vm shutdown
>> ...
>>    some other operations
>> ...
>> 6. vm shutdown
>> 7. start vm in stopped mode, and remove all bitmaps marked for removing
>> 8. stop vm
>>
>> But, I think that in real circumstances, vm shutdown is rare thing...
> 
> This is sounding a bit more serious. As I said earlier, it shouldn't
> delay 4.2 on its own, but if the fix is obvious (and other than
> comments, it is a single change from 'ret = -EINVAL' to 'ret = 0' which
> fixes a definite reproducible crash), I think it rises to the level of
> acceptable.
> 
> I've been so worried about the question of which release, that I don't
> know if I've previously offered:
> Reviewed-by: Eric Blake 
> 

Oh, that is quite a bit more serious than I thought too.

Yeah, I want this in 4.2 if at all possible.

Reviewed-by: John Snow 

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

[libvirt] [jenkins-ci PATCH v2 3/3] guests: add libprlsdk package to libvirt project

[Daniel P . Berrangé]

Signed-off-by: Daniel P. Berrangé 
---
 guests/vars/projects/libvirt.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/guests/vars/projects/libvirt.yml b/guests/vars/projects/libvirt.yml
index 780a5aa..1efa846 100644
--- a/guests/vars/projects/libvirt.yml
+++ b/guests/vars/projects/libvirt.yml
@@ -31,6 +31,7 @@ packages:
   - libparted
   - libpcap
   - libpciaccess
+  - libprlsdk
   - librbd
   - libselinux
   - libssh
-- 
2.23.0

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

[libvirt] [jenkins-ci PATCH v2 1/3] guests: add openvz repository on CentOS 7

[Daniel P . Berrangé]

The OpenVZ site provides a yum repo built against RHEL-7 that includes
the prlsdk-devel RPM needed for the VZ driver. This repo has quite alot
of packages that replace stuff from standard RHEL repos, so the yum
config file is set to whitelist only the minimal RPMs we need to do
builds. Fortunately they have no deps which would cause replacement of
standard RHEL RPMs.

Note this does not use the latest OpenVZ repo link, since that currently
has broken dependencies present

Error: Package: libprlcommon-7.0.183-1.vz7.x86_64 (vz)
   Requires: libjson-c.so.2(libjson-c.so.2)(64bit)

The Requires line ought to be

   libjson-c.so.2()(64bit)

Once that's fixed we can switch to the latest repo link.

Signed-off-by: Daniel P. Berrangé 
---
 guests/lcitool| 22 
 guests/playbooks/update/tasks/base.yml| 25 +++
 guests/playbooks/update/templates/openvz.key  | 20 +++
 .../playbooks/update/templates/openvz.repo.j2 |  9 +++
 4 files changed, 76 insertions(+)
 create mode 100644 guests/playbooks/update/templates/openvz.key
 create mode 100644 guests/playbooks/update/templates/openvz.repo.j2

diff --git a/guests/lcitool b/guests/lcitool
index d617beb..4f874b3 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -593,6 +593,18 @@ class Application:
 self._execute_playbook("build", args.hosts, args.projects,
args.git_revision)
 
+def _get_openvz_repo(self):
+basedir = os.path.dirname(sys.argv[0])
+repofile = os.path.join(basedir, "playbooks", "update", "templates", 
"openvz.repo.j2")
+with open(repofile, "r") as r:
+return r.read().rstrip()
+
+def _get_openvz_key(self):
+basedir = os.path.dirname(sys.argv[0])
+repofile = os.path.join(basedir, "playbooks", "update", "templates", 
"openvz.key")
+with open(repofile, "r") as r:
+return r.read().rstrip()
+
 def _action_dockerfile(self, args):
 mappings = self._projects.get_mappings()
 pip_mappings = self._projects.get_pip_mappings()
@@ -723,6 +735,16 @@ class Application:
 {package_manager} clean all -y
 """).format(**varmap))
 elif os_name == "CentOS" and os_version == "7":
+repo = self._get_openvz_repo()
+repocmd = "\\n\\\n".join(repo.split("\n"))
+key = self._get_openvz_key()
+keycmd = "\\n\\\n".join(key.split("\n"))
+
+sys.stdout.write(
+"RUN echo -e '%s' > /etc/yum.repos.d/openvz.repo && \\\n" 
% repocmd +
+"echo -e '%s' > /etc/pki/rpm-gpg/RPM-GPG-KEY-OpenVZ && 
\\\n" % keycmd +
+"rpm --import /etc/pki/rpm-gpg/RPM-GPG-KEY-OpenVZ")
+
 sys.stdout.write(textwrap.dedent("""
 RUN {package_manager} update -y && \\
 {package_manager} install -y epel-release && \\
diff --git a/guests/playbooks/update/tasks/base.yml 
b/guests/playbooks/update/tasks/base.yml
index 3d83e78..e17b50b 100644
--- a/guests/playbooks/update/tasks/base.yml
+++ b/guests/playbooks/update/tasks/base.yml
@@ -13,6 +13,31 @@
   package:
 name: epel-release
 state: latest
+
+- name: Create OpenVZ key
+  template:
+src: '{{ playbook_base }}/templates/openvz.key'
+dest: /etc/pki/rpm-gpg/RPM-GPG-KEY-OpenVZ
+owner: root
+group: root
+  when:
+- os_name == 'CentOS'
+- os_version == '7'
+
+- name: Import OpenVZ key
+  command: 'rpm --import /etc/pki/rpm-gpg/RPM-GPG-KEY-OpenVZ'
+  args:
+warn: no
+  when:
+- os_name == 'CentOS'
+- os_version == '7'
+
+- name: Enable OpenVZ repository
+  template:
+src: '{{ playbook_base }}/templates/openvz.repo.j2'
+dest: /etc/yum.repos.d/openvz.repo
+owner: root
+group: root
   when:
 - os_name == 'CentOS'
 - os_version == '7'
diff --git a/guests/playbooks/update/templates/openvz.key 
b/guests/playbooks/update/templates/openvz.key
new file mode 100644
index 000..b77a137
--- /dev/null
+++ b/guests/playbooks/update/templates/openvz.key
@@ -0,0 +1,20 @@
+-BEGIN PGP PUBLIC KEY BLOCK-
+Version: GnuPG v2.0.22 (GNU/Linux)
+
+mI0EVl80nQEEAKrEeyeTCwrzS9kYedZ/sAc/GUqlb81C7pA9SaR3fyck5mVw1Ogk
+YdmNBPM2kY7QDxR9F0EpSpnxSCAXZXugsQ8KzZ0DRLVeBDQyGs9IGK5hI0zzxIil
+BzfvIexLiQQhLy7YlIi8Jt/uUqKkW0pIMNMGcduY97VATtczpncpkmSzABEBAAG0
+SFZpcnR1b3p6byBUZWFtIChHUEcga2V5IHNpZ25hdHVyZSBmb3IgcGFja2FnZXMp
+IDxzZWN1cml0eUB2aXJ0dW96em8uY29tPoi5BBMBAgAjBQJWXzSdAhsDBwsJCAcD
+AgEGFQgCCQoLBBYCAwECHgECF4AACgkQygt9GUTNrSruIgP/er70Eyo73A1gfrjv
+oPUkyo4rslVRZu3qqCwoMFtJc/Z/UxWgEka1buorlcGLa6eO/EZ49c0n+KGa4Kvt
+EUboIq0yEu5i0FyAj92ifm+hNhoAbGfm0cZ4/fD0oGr3l8OsQo4+iHX4xAPwFe7Y
+zABuB8I1ZDZ4OIp5tDfTTuF2LT24jQRWXzSdAQQAog2Aqb+Ptl68O7cQhWLjVGkj
+yyigZrdeReLx3HloKJPBeQ/kA6uvMJc/IYS3uppMWXv9v+QenS6uhP1TUJ2k9FvM
+t94MQZfALN7Vpf8AF+UeWu4Ru+y4BNzcFhrPhIFNFChOR2QqW6FkgE57D9I177

[libvirt] [jenkins-ci PATCH v2 2/3] guests: define mapping for the libprlsdk package

[Daniel P . Berrangé]

Add a mapping exclusively for CentOS 7 to pull in the libprlsdk package,
since other distros don't have it available at this time.

Signed-off-by: Daniel P. Berrangé 
---
 guests/vars/mappings.yml | 4 
 1 file changed, 4 insertions(+)

diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml
index f5dab6a..ce1294c 100644
--- a/guests/vars/mappings.yml
+++ b/guests/vars/mappings.yml
@@ -366,6 +366,10 @@ mappings:
 rpm: libpciaccess-devel
 cross-policy-deb: foreign
 
+  libprlsdk:
+default:
+CentOS7: libprlsdk-devel
+
   librbd:
 deb: librbd-dev
 Fedora: librbd-devel
-- 
2.23.0

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

[libvirt] [jenkins-ci PATCH v2 0/3] Enable building of the VZ driver on CentOS 7

[Daniel P . Berrangé]

The VZ driver in libvirt periodically gets broken by refactoring in
libvirt. This is not noticed either before or after merge because none
of our CI tests nor common developer build hosts includ the deps needed
for the VZ driver.

The OpenVZ project, however, does provide builds of the required
packages for RHEL-7. We can use these packages in our CentOS 7 CI VMs to
enable build testing of the VZ driver. This closes the only hole we have
in driver build coverage for CI.

Changed in v2:

 - Enable GPG verification
 - Use older repo avoid temporary deps problem
 - Setup repos in dockerfile too.

Daniel P. Berrangé (3):
  guests: add openvz repository on CentOS 7
  guests: define mapping for the libprlsdk package
  guests: add libprlsdk package to libvirt project

 guests/lcitool| 22 
 guests/playbooks/update/tasks/base.yml| 25 +++
 guests/playbooks/update/templates/openvz.key  | 20 +++
 .../playbooks/update/templates/openvz.repo.j2 |  9 +++
 guests/vars/mappings.yml  |  4 +++
 guests/vars/projects/libvirt.yml  |  1 +
 6 files changed, 81 insertions(+)
 create mode 100644 guests/playbooks/update/templates/openvz.key
 create mode 100644 guests/playbooks/update/templates/openvz.repo.j2

-- 
2.23.0

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

[libvirt] [PATCH 5/8] qemu: blockjob: Allow NULL 'mirror' for block copy jobs due to migration

[Peter Krempa]

The non-shared-storage migration tracks the storage source used
explicitly in the migration data so we must allow for processing of the
block job which has NULL mirror as the mirror will not be populated.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_blockjob.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index baa79ea80c..2773acc990 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -1139,7 +1139,9 @@ 
qemuBlockJobProcessEventConcludedCopyPivot(virQEMUDriverPtr driver,
 {
 VIR_DEBUG("copy job '%s' on VM '%s' pivoted", job->name, vm->def->name);

-if (!job->disk)
+/* mirror may be NULL for copy job corresponding to migragion */
+if (!job->disk ||
+!job->disk->mirror)
 return;

 /* for shallow copy without reusing external image the user can either not
@@ -1166,7 +1168,9 @@ 
qemuBlockJobProcessEventConcludedCopyAbort(virQEMUDriverPtr driver,
 {
 VIR_DEBUG("copy job '%s' on VM '%s' aborted", job->name, vm->def->name);

-if (!job->disk)
+/* mirror may be NULL for copy job corresponding to migragion */
+if (!job->disk ||
+!job->disk->mirror)
 return;

 qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob, 
job->disk->mirror);
@@ -1383,7 +1387,8 @@ qemuBlockJobEventProcess(virQEMUDriverPtr driver,
 break;

 case QEMU_BLOCKJOB_STATE_READY:
-if (job->disk && job->disk->mirror) {
+/* mirror may be NULL for copy job corresponding to migragion */
+if (job->disk) {
 job->disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_READY;
 qemuBlockJobEmitEvents(driver, vm, job->disk, job->type, 
job->newstate);
 }
-- 
2.23.0

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

[libvirt] [PATCH 6/8] qemu: migration: Split out setup of the migration target

[Peter Krempa]

Separate out allocation of the virStorageSource corresponding to the
target NBD export of the migration.

As part of the splitout we allocate the export name explicitly as that
one must not change regardless whether blockdev is used or not to
provide compatibility.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_migration.c | 55 +--
 1 file changed, 35 insertions(+), 20 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 9062be38ab..8bf23e9f30 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -776,38 +776,28 @@ qemuMigrationSrcNBDCopyCancel(virQEMUDriverPtr driver,
 }


-static int
-qemuMigrationSrcNBDStorageCopyBlockdev(virQEMUDriverPtr driver,
-   virDomainObjPtr vm,
-   virDomainDiskDefPtr disk,
-   const char *diskAlias,
-   const char *host,
-   int port,
-   unsigned long long mirror_speed,
-   unsigned int mirror_shallow,
-   const char *tlsAlias)
+static virStorageSourcePtr
+qemuMigrationSrcNBDStorageCopyBlockdevPrepareSource(virDomainDiskDefPtr disk,
+const char *host,
+int port,
+const char *tlsAlias)
 {
-g_autoptr(qemuBlockStorageSourceAttachData) data = NULL;
-qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
-int mon_ret = 0;
 g_autoptr(virStorageSource) copysrc = NULL;

-VIR_DEBUG("starting blockdev mirror for disk=%s to host=%s", diskAlias, 
host);
-
 if (!(copysrc = virStorageSourceNew()))
-return -1;
+return NULL;

 copysrc->type = VIR_STORAGE_TYPE_NETWORK;
 copysrc->protocol = VIR_STORAGE_NET_PROTOCOL_NBD;
 copysrc->format = VIR_STORAGE_FILE_RAW;

 if (!(copysrc->backingStore = virStorageSourceNew()))
-return -1;
+return NULL;

-copysrc->path = g_strdup(diskAlias);
+if (!(copysrc->path = qemuAliasDiskDriveFromDisk(disk)))
+return NULL;

-if (VIR_ALLOC_N(copysrc->hosts, 1) < 0)
-return -1;
+copysrc->hosts = g_new0(virStorageNetHostDef, 1);

 copysrc->nhosts = 1;
 copysrc->hosts->transport = VIR_STORAGE_NET_HOST_TRANS_TCP;
@@ -819,6 +809,31 @@ qemuMigrationSrcNBDStorageCopyBlockdev(virQEMUDriverPtr 
driver,
 copysrc->nodestorage = g_strdup_printf("migration-%s-storage", disk->dst);
 copysrc->nodeformat = g_strdup_printf("migration-%s-format", disk->dst);

+return g_steal_pointer(©src);
+}
+
+
+static int
+qemuMigrationSrcNBDStorageCopyBlockdev(virQEMUDriverPtr driver,
+   virDomainObjPtr vm,
+   virDomainDiskDefPtr disk,
+   const char *diskAlias,
+   const char *host,
+   int port,
+   unsigned long long mirror_speed,
+   unsigned int mirror_shallow,
+   const char *tlsAlias)
+{
+g_autoptr(qemuBlockStorageSourceAttachData) data = NULL;
+qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+int mon_ret = 0;
+g_autoptr(virStorageSource) copysrc = NULL;
+
+VIR_DEBUG("starting blockdev mirror for disk=%s to host=%s", diskAlias, 
host);
+
+if (!(copysrc = qemuMigrationSrcNBDStorageCopyBlockdevPrepareSource(disk, 
host, port, tlsAlias)))
+return -1;
+
 /* Migration via blockdev-mirror was supported sooner than the 
auto-read-only
  * feature was added to qemu */
 if (!(data = qemuBlockStorageSourceAttachPrepareBlockdev(copysrc,
-- 
2.23.0

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

[libvirt] [PATCH 3/8] qemu: migration: Access job name from job struct

[Peter Krempa]

qemuMigrationSrcNBDCopyCancelOne uses the block job data structure but
generated it's own job name rather than taking it from the block job
data.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_migration.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 2416dbe432..b14a1bcd15 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -649,7 +649,6 @@ qemuMigrationSrcNBDCopyCancelOne(virQEMUDriverPtr driver,
  qemuDomainAsyncJob asyncJob)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
-char *diskAlias = NULL;
 int ret = -1;
 int status;
 int rv;
@@ -668,13 +667,10 @@ qemuMigrationSrcNBDCopyCancelOne(virQEMUDriverPtr driver,
 goto cleanup;
 }

-if (!(diskAlias = qemuAliasDiskDriveFromDisk(disk)))
-return -1;
-
 if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
 goto cleanup;

-rv = qemuMonitorBlockJobCancel(priv->mon, diskAlias);
+rv = qemuMonitorBlockJobCancel(priv->mon, job->name);

 if (qemuDomainObjExitMonitor(driver, vm) < 0 || rv < 0)
 goto cleanup;
@@ -682,7 +678,6 @@ qemuMigrationSrcNBDCopyCancelOne(virQEMUDriverPtr driver,
 ret = 0;

  cleanup:
-VIR_FREE(diskAlias);
 return ret;
 }

-- 
2.23.0

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

[libvirt] [PATCH 7/8] qemu: migration: Mention disk target rather than the drive name in debug msg

[Peter Krempa]

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_migration.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 8bf23e9f30..27023c94b1 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -829,7 +829,7 @@ qemuMigrationSrcNBDStorageCopyBlockdev(virQEMUDriverPtr 
driver,
 int mon_ret = 0;
 g_autoptr(virStorageSource) copysrc = NULL;

-VIR_DEBUG("starting blockdev mirror for disk=%s to host=%s", diskAlias, 
host);
+VIR_DEBUG("starting blockdev mirror for disk=%s to host=%s", disk->dst, 
host);

 if (!(copysrc = qemuMigrationSrcNBDStorageCopyBlockdevPrepareSource(disk, 
host, port, tlsAlias)))
 return -1;
-- 
2.23.0

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

[libvirt] [PATCH 4/8] qemu: migration: Simplify cleanup in qemuMigrationSrcNBDCopyCancelOne

[Peter Krempa]

Now that the cleanup section does not exist remove the label.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_migration.c | 15 +--
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index b14a1bcd15..9062be38ab 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -649,7 +649,6 @@ qemuMigrationSrcNBDCopyCancelOne(virQEMUDriverPtr driver,
  qemuDomainAsyncJob asyncJob)
 {
 qemuDomainObjPrivatePtr priv = vm->privateData;
-int ret = -1;
 int status;
 int rv;

@@ -659,26 +658,22 @@ qemuMigrationSrcNBDCopyCancelOne(virQEMUDriverPtr driver,
 case VIR_DOMAIN_BLOCK_JOB_CANCELED:
 if (failNoJob) {
 qemuMigrationNBDReportMirrorError(job, disk->dst);
-goto cleanup;
+return -1;
 }
 G_GNUC_FALLTHROUGH;
 case VIR_DOMAIN_BLOCK_JOB_COMPLETED:
-ret = 1;
-goto cleanup;
+return 1;
 }

 if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
-goto cleanup;
+return -1;

 rv = qemuMonitorBlockJobCancel(priv->mon, job->name);

 if (qemuDomainObjExitMonitor(driver, vm) < 0 || rv < 0)
-goto cleanup;
-
-ret = 0;
+return -1;

- cleanup:
-return ret;
+return 0;
 }


-- 
2.23.0

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

[libvirt] [PATCH 1/8] qemu: migration: Simplify handling of 'diskAlias' when adding NBD exports

[Peter Krempa]

Declare the variable inside the loop with automatic clearing.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_migration.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index dabdda2715..af22dfb48d 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -377,7 +377,6 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver,
 int ret = -1;
 qemuDomainObjPrivatePtr priv = vm->privateData;
 unsigned short port = 0;
-char *diskAlias = NULL;
 size_t i;
 virStorageNetHostDef server = {
 .name = (char *)listenAddr, /* cast away const */
@@ -392,6 +391,7 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver,

 for (i = 0; i < vm->def->ndisks; i++) {
 virDomainDiskDefPtr disk = vm->def->disks[i];
+g_autofree char *diskAlias = NULL;

 /* check whether disk should be migrated */
 if (!qemuMigrationAnyCopyDisk(disk, nmigrate_disks, migrate_disks))
@@ -404,7 +404,6 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver,
 goto cleanup;
 }

-VIR_FREE(diskAlias);
 if (!(diskAlias = qemuAliasDiskDriveFromDisk(disk)))
 goto cleanup;

@@ -433,7 +432,6 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver,
 ret = 0;

  cleanup:
-VIR_FREE(diskAlias);
 if (ret < 0 && nbdPort == 0)
 virPortAllocatorRelease(port);
 return ret;
-- 
2.23.0

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

[libvirt] [PATCH 8/8] qemu: migration: Properly setup mirror for blockdev configurations

[Peter Krempa]

With blockdev we need to refer to the nodename of the disk source image
as the source argument for the blockdev-mirror operation while still
keeping the old job name. With blockdev we must also persist the job in
qemu.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_migration.c | 24 
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 27023c94b1..6dfcd5a642 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -817,7 +817,9 @@ static int
 qemuMigrationSrcNBDStorageCopyBlockdev(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk,
-   const char *diskAlias,
+   const char *jobname,
+   const char *sourcename,
+   bool persistjob,
const char *host,
int port,
unsigned long long mirror_speed,
@@ -848,8 +850,8 @@ qemuMigrationSrcNBDStorageCopyBlockdev(virQEMUDriverPtr 
driver,
 mon_ret = qemuBlockStorageSourceAttachApply(qemuDomainGetMonitor(vm), 
data);

 if (mon_ret == 0)
-mon_ret = qemuMonitorBlockdevMirror(qemuDomainGetMonitor(vm), NULL, 
false,
-diskAlias, copysrc->nodeformat,
+mon_ret = qemuMonitorBlockdevMirror(qemuDomainGetMonitor(vm), jobname, 
persistjob,
+sourcename, copysrc->nodeformat,
 mirror_speed, 0, 0, 
mirror_shallow);

 if (mon_ret != 0)
@@ -914,6 +916,9 @@ qemuMigrationSrcNBDStorageCopyOne(virQEMUDriverPtr driver,
 qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
 qemuBlockJobDataPtr job = NULL;
 char *diskAlias = NULL;
+const char *jobname = NULL;
+const char *sourcename = NULL;
+bool persistjob = false;
 int rc;
 int ret = -1;

@@ -923,12 +928,23 @@ qemuMigrationSrcNBDStorageCopyOne(virQEMUDriverPtr driver,
 if (!(job = qemuBlockJobDiskNew(vm, disk, QEMU_BLOCKJOB_TYPE_COPY, 
diskAlias)))
 goto cleanup;

+if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) {
+jobname = diskAlias;
+sourcename = disk->src->nodeformat;
+persistjob = true;
+} else {
+jobname = NULL;
+sourcename = diskAlias;
+persistjob = false;
+}
+
 qemuBlockJobSyncBegin(job);

 if (flags & VIR_MIGRATE_TLS ||
 virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) {
 rc = qemuMigrationSrcNBDStorageCopyBlockdev(driver, vm,
-disk, diskAlias,
+disk, jobname,
+sourcename, persistjob,
 host, port,
 mirror_speed,
 mirror_shallow,
-- 
2.23.0

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

[libvirt] [PATCH 2/8] qemu: migration: Properly export backend for NBD storage migration

[Peter Krempa]

With -blockdev we must use the nodename as the export but we must keep
the name of the export as it was before to ensure compatiblity.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_migration.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index af22dfb48d..2416dbe432 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -392,6 +392,8 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver,
 for (i = 0; i < vm->def->ndisks; i++) {
 virDomainDiskDefPtr disk = vm->def->disks[i];
 g_autofree char *diskAlias = NULL;
+const char *exportname = NULL;
+const char *devicename = NULL;

 /* check whether disk should be migrated */
 if (!qemuMigrationAnyCopyDisk(disk, nmigrate_disks, migrate_disks))
@@ -407,6 +409,14 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver,
 if (!(diskAlias = qemuAliasDiskDriveFromDisk(disk)))
 goto cleanup;

+if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) {
+exportname = diskAlias;
+devicename = disk->src->nodeformat;
+} else {
+exportname = NULL;
+devicename = diskAlias;
+}
+
 if (qemuDomainObjEnterMonitorAsync(driver, vm,
QEMU_ASYNC_JOB_MIGRATION_IN) < 0)
 goto cleanup;
@@ -422,7 +432,7 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver,
 goto exit_monitor;
 }

-if (qemuMonitorNBDServerAdd(priv->mon, diskAlias, NULL, true, NULL) < 
0)
+if (qemuMonitorNBDServerAdd(priv->mon, devicename, exportname, true, 
NULL) < 0)
 goto exit_monitor;
 if (qemuDomainObjExitMonitor(driver, vm) < 0)
 goto cleanup;
-- 
2.23.0

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

[libvirt] [PATCH 0/8] qemu: Fix non-shared storage migration over NBD (blockdev-add sequels)

[Peter Krempa]

I put some blockdev awareness into the non-shared storage migration NBD
code but it was not complete. Seeing it there mislead me thinking that
it's done.

Fix it now.

Peter Krempa (8):
  qemu: migration: Simplify handling of 'diskAlias' when adding NBD
exports
  qemu: migration: Properly export backend for NBD storage migration
  qemu: migration: Access job name from job struct
  qemu: migration: Simplify cleanup in qemuMigrationSrcNBDCopyCancelOne
  qemu: blockjob: Allow NULL 'mirror' for block copy jobs due to
migration
  qemu: migration: Split out setup of the migration target
  qemu: migration: Mention disk target rather than the drive name in
debug msg
  qemu: migration: Properly setup mirror for blockdev configurations

 src/qemu/qemu_blockjob.c  |  11 +++-
 src/qemu/qemu_migration.c | 115 --
 2 files changed, 80 insertions(+), 46 deletions(-)

-- 
2.23.0

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

[libvirt] [PATCH] docs: switch to use rst2html5 instead of rst2html

[Daniel P . Berrangé]

Now that we don't have to support CentOS 7's python2-docutils, we can
use the rst2html5 tool instead, which generates HTML5 output instead
of HTML4 output. This matches what we used for the original HTML docs
since

  commit b1c81567c7172bc9dcd701cf46ea3f87725d62c7
  Author: Daniel P. Berrangé 
  Date:   Wed Jul 26 18:01:25 2017 +0100

docs: switch to using HTML5 doctype declaration

Signed-off-by: Daniel P. Berrangé 
---
 m4/virt-external-programs.m4 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/m4/virt-external-programs.m4 b/m4/virt-external-programs.m4
index ed634a4c73..a23c69d54b 100644
--- a/m4/virt-external-programs.m4
+++ b/m4/virt-external-programs.m4
@@ -33,7 +33,7 @@ AC_DEFUN([LIBVIRT_CHECK_EXTERNAL_PROGRAMS], [
   then
 AC_MSG_ERROR("xsltproc is required to build libvirt")
   fi
-  AC_PATH_PROGS([RST2HTML], [rst2html rst2html.py rst2html-3], [])
+  AC_PATH_PROGS([RST2HTML], [rst2html5 rst2html5.py rst2html5-3], [])
   if test -z "$RST2HTML"
   then
 AC_MSG_ERROR("rst2html is required to build libvirt")
-- 
2.23.0

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

Re: [libvirt] [RFC PATCH 0/9] Introduce mediate ops in vfio-pci

[Alex Williamson]

On Fri, 6 Dec 2019 17:40:02 +0800
Jason Wang  wrote:

> On 2019/12/6 下午4:22, Yan Zhao wrote:
> > On Thu, Dec 05, 2019 at 09:05:54PM +0800, Jason Wang wrote:  
> >> On 2019/12/5 下午4:51, Yan Zhao wrote:  
> >>> On Thu, Dec 05, 2019 at 02:33:19PM +0800, Jason Wang wrote:  
>  Hi:
> 
>  On 2019/12/5 上午11:24, Yan Zhao wrote:  
> > For SRIOV devices, VFs are passthroughed into guest directly without 
> > host
> > driver mediation. However, when VMs migrating with passthroughed VFs,
> > dynamic host mediation is required to  (1) get device states, (2) get
> > dirty pages. Since device states as well as other critical information
> > required for dirty page tracking for VFs are usually retrieved from PFs,
> > it is handy to provide an extension in PF driver to centralizingly 
> > control
> > VFs' migration.
> >
> > Therefore, in order to realize (1) passthrough VFs at normal time, (2)
> > dynamically trap VFs' bars for dirty page tracking and  
>  A silly question, what's the reason for doing this, is this a must for 
>  dirty
>  page tracking?
>   
> >>> For performance consideration. VFs' bars should be passthoughed at
> >>> normal time and only enter into trap state on need.  
> >>
> >> Right, but how does this matter for the case of dirty page tracking?
> >>  
> > Take NIC as an example, to trap its VF dirty pages, software way is
> > required to trap every write of ring tail that resides in BAR0.  
> 
> 
> Interesting, but it looks like we need:
> - decode the instruction
> - mediate all access to BAR0
> All of which seems a great burden for the VF driver. I wonder whether or 
> not doing interrupt relay and tracking head is better in this case.

This sounds like a NIC specific solution, I believe the goal here is to
allow any device type to implement a partial mediation solution, in
this case to sufficiently track the device while in the migration
saving state.

> >   There's
> > still no IOMMU Dirty bit available.  
> > (3) centralizing
> > VF critical states retrieving and VF controls into one driver, we 
> > propose
> > to introduce mediate ops on top of current vfio-pci device driver.
> >
> >
> >   _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
> > __   register mediate ops|  ___ ___|
> > |  |<---| VF|   |   |
> > | vfio-pci |  | |  mediate  |   | PF driver |   |
> > |__|--->|   driver  |   |___|
> > |open(pdev)  |  ---  | |
> > ||
> > ||_ _ _ _ _ _ _ _ _ _ _ _|_ _ _ _ _|
> >\|/  \|/
> > --- 
> > |VF   | |PF|
> > --- 
> >
> >
> > VF mediate driver could be a standalone driver that does not bind to
> > any devices (as in demo code in patches 5-6) or it could be a built-in
> > extension of PF driver (as in patches 7-9) .
> >
> > Rather than directly bind to VF, VF mediate driver register a mediate
> > ops into vfio-pci in driver init. vfio-pci maintains a list of such
> > mediate ops.
> > (Note that: VF mediate driver can register mediate ops into vfio-pci
> > before vfio-pci binding to any devices. And VF mediate driver can
> > support mediating multiple devices.)
> >
> > When opening a device (e.g. a VF), vfio-pci goes through the mediate ops
> > list and calls each vfio_pci_mediate_ops->open() with pdev of the 
> > opening
> > device as a parameter.
> > VF mediate driver should return success or failure depending on it
> > supports the pdev or not.
> > E.g. VF mediate driver would compare its supported VF devfn with the
> > devfn of the passed-in pdev.
> > Once vfio-pci finds a successful vfio_pci_mediate_ops->open(), it will
> > stop querying other mediate ops and bind the opening device with this
> > mediate ops using the returned mediate handle.
> >
> > Further vfio-pci ops (VFIO_DEVICE_GET_REGION_INFO ioctl, rw, mmap) on 
> > the
> > VF will be intercepted into VF mediate driver as
> > vfio_pci_mediate_ops->get_region_info(),
> > vfio_pci_mediate_ops->rw,
> > vfio_pci_mediate_ops->mmap, and get customized.
> > For vfio_pci_mediate_ops->rw and vfio_pci_mediate_ops->mmap, they will
> > further return 'pt' to indicate whether vfio-pci should further
> > passthrough data to hw.
> >
> > when vfio-pci closes the VF, it calls its 
> > vfio_pci_mediate_ops->release()
> > with a mediate ha

[libvirt] [PATCH] docs: fix duplication variable name for rst files

[Daniel P . Berrangé]

Signed-off-by: Daniel P. Berrangé 
---
 docs/Makefile.am | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/docs/Makefile.am b/docs/Makefile.am
index 5f5dce28eb..9462f77458 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -173,10 +173,10 @@ gif = \
 
 internals_html_in = \
   $(patsubst $(srcdir)/%,%,$(wildcard $(srcdir)/internals/*.html.in))
-kbase_rst = \
-  $(patsubst $(srcdir)/%,%,$(wildcard $(srcdir)/kbase/*.rst))
-kbase_rst_html_in = \
-  $(kbase_rst:%.rst=%.html.in)
+internals_rst = \
+  $(patsubst $(srcdir)/%,%,$(wildcard $(srcdir)/internals/*.rst))
+internals_rst_html_in = \
+  $(internals_rst:%.rst=%.html.in)
 internals_html = \
   $(internals_html_in:%.html.in=%.html) \
   $(internals_rst_html_in:%.html.in=%.html)
-- 
2.23.0

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

[libvirt] [PATCH] scripts: ignore remote protocol checks if pdwtags crashes

[Daniel P . Berrangé]

On Debian 10, pdwtags reliably segfaults when parsing the libvirt remote
protocol files. This crash was previously ignored by 'make check'
because of the way we piped the pdwtags output to the perl
post-processing scripts. When this was converted to use python it
mistakenly started being a fatal error. We need to explicitly ignore
pdwtags output if it exited with non-zero return code.

Signed-off-by: Daniel P. Berrangé 
---
 scripts/check-remote-protocol.py | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/scripts/check-remote-protocol.py b/scripts/check-remote-protocol.py
index 201166fd9e..25caa19563 100644
--- a/scripts/check-remote-protocol.py
+++ b/scripts/check-remote-protocol.py
@@ -86,8 +86,12 @@ out, err = pdwtagsproc.communicate()
 out = out.decode("utf-8")
 err = err.decode("utf-8")
 
-if out == "" and err != "":
-print("WARNING: no output, pdwtags appears broken:", file=sys.stderr)
+if out == "" or pdwtagsproc.returncode != 0:
+if out == "":
+print("WARNING: no output, pdwtags appears broken:", file=sys.stderr)
+else:
+print("WARNING: exit code %d, pdwtags appears broken:" %
+  pdwtagsproc.returncode, file=sys.stderr)
 for l in err.strip().split("\n"):
 print("WARNING: %s" % l, file=sys.stderr)
 print("WARNING: skipping the remote protocol test", file=sys.stderr)
-- 
2.23.0

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

Re: [libvirt] [RFC PATCH 4/9] vfio-pci: register default dynamic-trap-bar-info region

[Alex Williamson]

On Fri, 6 Dec 2019 01:04:07 -0500
Yan Zhao  wrote:

> On Fri, Dec 06, 2019 at 07:55:30AM +0800, Alex Williamson wrote:
> > On Wed,  4 Dec 2019 22:26:50 -0500
> > Yan Zhao  wrote:
> >   
> > > Dynamic trap bar info region is a channel for QEMU and vendor driver to
> > > communicate dynamic trap info. It is of type
> > > VFIO_REGION_TYPE_DYNAMIC_TRAP_BAR_INFO and subtype
> > > VFIO_REGION_SUBTYPE_DYNAMIC_TRAP_BAR_INFO.
> > > 
> > > This region has two fields: dt_fd and trap.
> > > When QEMU detects a device regions of this type, it will create an
> > > eventfd and write its eventfd id to dt_fd field.
> > > When vendor drivre signals this eventfd, QEMU reads trap field of this
> > > info region.
> > > - If trap is true, QEMU would search the device's PCI BAR
> > > regions and disable all the sparse mmaped subregions (if the sparse
> > > mmaped subregion is disablable).
> > > - If trap is false, QEMU would re-enable those subregions.
> > > 
> > > A typical usage is
> > > 1. vendor driver first cuts its bar 0 into several sections, all in a
> > > sparse mmap array. So initally, all its bar 0 are passthroughed.
> > > 2. vendor driver specifys part of bar 0 sections to be disablable.
> > > 3. on migration starts, vendor driver signals dt_fd and set trap to true
> > > to notify QEMU disabling the bar 0 sections of disablable flags on.
> > > 4. QEMU disables those bar 0 section and hence let vendor driver be able
> > > to trap access of bar 0 registers and make dirty page tracking possible.
> > > 5. on migration failure, vendor driver signals dt_fd to QEMU again.
> > > QEMU reads trap field of this info region which is false and QEMU
> > > re-passthrough the whole bar 0 region.
> > > 
> > > Vendor driver specifies whether it supports dynamic-trap-bar-info region
> > > through cap VFIO_PCI_DEVICE_CAP_DYNAMIC_TRAP_BAR in
> > > vfio_pci_mediate_ops->open().
> > > 
> > > If vfio-pci detects this cap, it will create a default
> > > dynamic_trap_bar_info region on behalf of vendor driver with region len=0
> > > and region->ops=null.
> > > Vvendor driver should override this region's len, flags, rw, mmap in its
> > > vfio_pci_mediate_ops.  
> > 
> > TBH, I don't like this interface at all.  Userspace doesn't pass data
> > to the kernel via INFO ioctls.  We have a SET_IRQS ioctl for
> > configuring user signaling with eventfds.  I think we only need to
> > define an IRQ type that tells the user to re-evaluate the sparse mmap
> > information for a region.  The user would enumerate the device IRQs via
> > GET_IRQ_INFO, find one of this type where the IRQ info would also
> > indicate which region(s) should be re-evaluated on signaling.  The user
> > would enable that signaling via SET_IRQS and simply re-evaluate the  
> ok. I'll try to switch to this way. Thanks for this suggestion.
> 
> > sparse mmap capability for the associated regions when signaled.  
> 
> Do you like the "disablable" flag of sparse mmap ?
> I think it's a lightweight way for user to switch mmap state of a whole 
> region,
> otherwise going through a complete flow of GET_REGION_INFO and re-setup
> region might be too heavy.

No, I don't like the disable-able flag.  At what frequency do we expect
regions to change?  It seems like we'd only change when switching into
and out of the _SAVING state, which is rare.  It seems easy for
userspace, at least QEMU, to drop the entire mmap configuration and
re-read it.  Another concern here is how do we synchronize the event?
Are we assuming that this event would occur when a user switch to
_SAVING mode on the device?  That operation is synchronous, the device
must be in saving mode after the write to device state completes, but
it seems like this might be trying to add an asynchronous dependency.
Will the write to device_state only complete once the user handles the
eventfd?  How would the kernel know when the mmap re-evaluation is
complete.  It seems like there are gaps here that the vendor driver
could miss traps required for migration because the user hasn't
completed the mmap transition yet.  Thanks,

Alex

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

Re: [libvirt] [PATCH 0/3] qemu: fix setting of block job speed (blockdev-add sequels)

[Eric Blake]

On 12/6/19 9:07 AM, Peter Krempa wrote:

See patch 3.

Peter Krempa (3):
   qemu: domain: Mention searched disk in error of qemuDomainDiskByName
   qemu: driver: Use qemuDomainDiskByName instead of virDomainDiskByName
   qemu: driver: Use appropriate job name when setting blockjob speed


Series:
Reviewed-by: Eric Blake 



  src/qemu/qemu_domain.c |  4 ++--
  src/qemu/qemu_driver.c | 20 
  2 files changed, 10 insertions(+), 14 deletions(-)



--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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

Re: [libvirt] [PATCH 2/2] net: Drop the NetLegacy structure, always use Netdev instead

[Eric Blake]

On 12/5/19 11:36 PM, Thomas Huth wrote:

Now that the "name" parameter is gone, there is hardly any difference
between NetLegacy and Netdev anymore. Drop NetLegacy and always use
Netdev to simplify the code quite a bit.

Signed-off-by: Thomas Huth 
---


Initial focus on the QAPI change:


+++ b/qapi/net.json
@@ -467,7 +467,7 @@
  # 'l2tpv3' - since 2.1
  ##
  { 'union': 'Netdev',
-  'base': { 'id': 'str', 'type': 'NetClientDriver' },
+  'base': { '*id': 'str', 'type': 'NetClientDriver' },


Making id optional here...


'discriminator': 'type',
'data': {
  'nic':  'NetLegacyNicOptions',
@@ -481,55 +481,6 @@
  'netmap':   'NetdevNetmapOptions',
  'vhost-user': 'NetdevVhostUserOptions' } }
  
-##

-# @NetLegacy:
-#
-# Captures the configuration of a network device; legacy.
-#
-# @id: identifier for monitor commands
-#
-# @opts: device type specific properties (legacy)
-#
-# Since: 1.2
-#
-# 'vlan': dropped in 3.0
-# 'name': dropped in 5.0
-##
-{ 'struct': 'NetLegacy',
-  'data': {
-'*id':   'str',
-'opts':  'NetLegacyOptions' } }


to match how it was here.  Should 'id' have been made mandatory in 1/2, 
when deleting 'name' (after all, id was optional only when name was in use)?



-
-##
-# @NetLegacyOptionsType:
-#
-# Since: 1.2
-##
-{ 'enum': 'NetLegacyOptionsType',
-  'data': ['none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde',
-   'bridge', 'netmap', 'vhost-user'] }


Comparing this to the branches of Netdev:

We are losing 'none', while gaining 'hubport'.  The gain is not 
problematic, and I guess you are declaring that the use of 'none' has 
been deprecated long enough to not be a problem.



-
-##
-# @NetLegacyOptions:
-#
-# Like Netdev, but for use only by the legacy command line options
-#
-# Since: 1.2
-##
-{ 'union': 'NetLegacyOptions',
-  'base': { 'type': 'NetLegacyOptionsType' },
-  'discriminator': 'type',
-  'data': {
-'nic':  'NetLegacyNicOptions',


Should we rename this to NetdevNicOptions, now that we are getting rid 
of other NetLegacy names?



-'user': 'NetdevUserOptions',
-'tap':  'NetdevTapOptions',
-'l2tpv3':   'NetdevL2TPv3Options',
-'socket':   'NetdevSocketOptions',
-'vde':  'NetdevVdeOptions',
-'bridge':   'NetdevBridgeOptions',
-'netmap':   'NetdevNetmapOptions',
-'vhost-user': 'NetdevVhostUserOptions' } }


But I concur that all branches of the Netdev union have the same types 
as what you are removing here from NetLegacyOptions, so the 
consolidation looks sane.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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

[libvirt] [PATCH 0/3] qemu: fix setting of block job speed (blockdev-add sequels)

[Peter Krempa]

See patch 3.

Peter Krempa (3):
  qemu: domain: Mention searched disk in error of qemuDomainDiskByName
  qemu: driver: Use qemuDomainDiskByName instead of virDomainDiskByName
  qemu: driver: Use appropriate job name when setting blockjob speed

 src/qemu/qemu_domain.c |  4 ++--
 src/qemu/qemu_driver.c | 20 
 2 files changed, 10 insertions(+), 14 deletions(-)

-- 
2.23.0

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

[libvirt] [PATCH 1/3] qemu: domain: Mention searched disk in error of qemuDomainDiskByName

[Peter Krempa]

Mention the argument used if the disk can't be located.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_domain.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 470d342afc..7e9b2db9eb 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -12491,8 +12491,8 @@ qemuDomainDiskByName(virDomainDefPtr def,
 virDomainDiskDefPtr ret;

 if (!(ret = virDomainDiskByName(def, name, true))) {
-virReportError(VIR_ERR_INVALID_ARG, "%s",
-   _("No device found for specified path"));
+virReportError(VIR_ERR_INVALID_ARG,
+   _("disk '%s' not found in domain"), name);
 return NULL;
 }

-- 
2.23.0

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

Re: [libvirt] [PATCH 1/2] net: Drop the legacy "name" parameter from the -net option

[Eric Blake]

On 12/5/19 11:36 PM, Thomas Huth wrote:

It's been deprecated since QEMU v3.1, so it's time to finally
remove it. The "id" parameter can simply be used instead.

Signed-off-by: Thomas Huth 
---
  net/net.c| 10 +-
  qapi/net.json|  4 +---
  qemu-deprecated.texi | 12 +++-
  3 files changed, 9 insertions(+), 17 deletions(-)



Reviewed-by: Eric Blake 

A quick grep of 'name.*netdev' and 'netdev.*name' in libvirt did not 
seem to find any uses of the deprecated syntax.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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

[libvirt] [PATCH 3/3] qemu: driver: Use appropriate job name when setting blockjob speed

[Peter Krempa]

qemuDomainBlockJobSetSpeed was not converted to get the job name from
the block job data. This means that after enabling blockdev the API call
would fail as we wouldn't use the appropriate name.

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

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_driver.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 82f70465bc..84c633aebf 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -17854,8 +17854,8 @@ qemuDomainBlockJobSetSpeed(virDomainPtr dom,
 virDomainDiskDefPtr disk;
 int ret = -1;
 virDomainObjPtr vm;
-g_autofree char *device = NULL;
 unsigned long long speed = bandwidth;
+g_autoptr(qemuBlockJobData) job = NULL;

 virCheckFlags(VIR_DOMAIN_BLOCK_JOB_SPEED_BANDWIDTH_BYTES, -1);

@@ -17885,12 +17885,15 @@ qemuDomainBlockJobSetSpeed(virDomainPtr dom,
 if (!(disk = qemuDomainDiskByName(vm->def, path)))
 goto endjob;

-if (!(device = qemuAliasDiskDriveFromDisk(disk)))
+if (!(job = qemuBlockJobDiskGetJob(disk))) {
+virReportError(VIR_ERR_INVALID_ARG,
+   _("disk %s does not have an active block job"), 
disk->dst);
 goto endjob;
+}

 qemuDomainObjEnterMonitor(driver, vm);
 ret = qemuMonitorBlockJobSetSpeed(qemuDomainGetMonitor(vm),
-  device,
+  job->name,
   speed);
 if (qemuDomainObjExitMonitor(driver, vm) < 0)
 ret = -1;
-- 
2.23.0

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

[libvirt] [PATCH 2/3] qemu: driver: Use qemuDomainDiskByName instead of virDomainDiskByName

[Peter Krempa]

Where appropriate replace the open coded call with the qemu wrapper
which already reports the error.

Signed-off-by: Peter Krempa 
---
 src/qemu/qemu_driver.c | 11 ++-
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index bad2fb52f3..82f70465bc 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11880,12 +11880,8 @@ qemuDomainBlockPeek(virDomainPtr dom,
 if (virDomainBlockPeekEnsureACL(dom->conn, vm->def) < 0)
 goto cleanup;

-/* Check the path belongs to this domain.  */
-if (!(disk = virDomainDiskByName(vm->def, path, true))) {
-virReportError(VIR_ERR_INVALID_ARG,
-   _("invalid disk or path '%s'"), path);
+if (!(disk = qemuDomainDiskByName(vm->def, path)))
 goto cleanup;
-}

 if (disk->src->format != VIR_STORAGE_FILE_RAW) {
 virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
@@ -17818,11 +17814,8 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom,
 if (virDomainObjCheckActive(vm) < 0)
 goto endjob;

-if (!(disk = virDomainDiskByName(vm->def, path, true))) {
-virReportError(VIR_ERR_INVALID_ARG,
-   _("disk %s not found in the domain"), path);
+if (!(disk = qemuDomainDiskByName(vm->def, path)))
 goto endjob;
-}

 if (!(job = qemuBlockJobDiskGetJob(disk))) {
 ret = 0;
-- 
2.23.0

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

[libvirt] [PATCH 17/17] docs: remove build recipes related to pod2man usage

[Daniel P . Berrangé]

Signed-off-by: Daniel P. Berrangé 
---
 src/Makefile.am   | 18 --
 tools/Makefile.am | 47 ---
 2 files changed, 65 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 239596624c..0267f16625 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -54,8 +54,6 @@ AM_LDFLAGS_MOD = \
$(AM_LDFLAGS)
 AM_LDFLAGS_MOD_NOUNDEF = $(AM_LDFLAGS_MOD) $(NO_UNDEFINED_LDFLAGS)
 
-POD2MAN = pod2man -c "Virtualization Support" -r "$(PACKAGE)-$(VERSION)"
-
 EXTRA_DIST = $(conf_DATA)
 
 BUILT_SOURCES =
@@ -84,15 +82,11 @@ endif WITH_DTRACE_PROBES
 libexec_PROGRAMS =
 RPC_PROBE_FILES =
 LOGROTATE_FILES_IN =
-PODFILES =
-MANINFILES =
 SYSTEMD_UNIT_FILES =
 SYSTEMD_UNIT_FILES_IN =
 SYSCONF_FILES =
 sbin_PROGRAMS =
-man8_MANS =
 DRIVER_SOURCES =
-man7_MANS =
 
 COMMON_UNIT_VARS = \
-e 's|[@]runstatedir[@]|$(runstatedir)|g' \
@@ -586,20 +580,8 @@ UNINSTALL_LOCAL += uninstall-logrotate
 endif WITH_LIBVIRTD
 
 
-%.8: %.8.in $(top_srcdir)/configure.ac
-   $(AM_V_GEN)sed \
-   -e 's|[@]sysconfdir[@]|$(sysconfdir)|g' \
-   -e 's|[@]runstatedir[@]|$(runstatedir)|g' \
-   < $< > $@-t && \
-   mv $@-t $@
-
-CLEANFILES += \
-   $(man8_MANS) \
-   $(MANINFILES)
-
 EXTRA_DIST += \
 $(SYSTEMD_UNIT_FILES_IN) \
-$(PODFILES) \
 $(NULL)
 
 
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 9565c6026f..350a6f3c84 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -52,12 +52,6 @@ ICON_FILES = \
libvirt_win_icon_64x64.ico \
virsh_win_icon.rc
 
-PODFILES = \
-   $(NULL)
-
-MANINFILES = \
-   $(NULL)
-
 EXTRA_DIST = \
$(ICON_FILES) \
$(conf_DATA) \
@@ -69,7 +63,6 @@ EXTRA_DIST = \
virsh-edit.c \
bash-completion/vsh \
libvirt_recover_xattrs.sh \
-   $(PODFILES) \
$(NULL)
 
 
@@ -83,7 +76,6 @@ conf_DATA =
 bin_SCRIPTS = virt-xml-validate virt-pki-validate
 bin_PROGRAMS = virsh virt-admin
 libexec_SCRIPTS = libvirt-guests.sh
-man1_MANS =
 
 if WITH_SANLOCK
 sbin_SCRIPTS = virt-sanlock-cleanup
@@ -300,42 +292,6 @@ virsh_win_icon.$(OBJEXT): virsh_win_icon.rc
  --output-format coff --output $@
 endif WITH_WIN_ICON
 
-POD2MAN = pod2man -c "Virtualization Support" -r "$(PACKAGE)-$(VERSION)"
-
-%.1.in: %.pod
-   $(AM_V_GEN)$(POD2MAN) $< $@-t1 && \
-   if grep 'POD ERROR' $@-t1; then rm $@-t1; exit 1; fi && \
-   sed \
-   -e 's|SYSCONFDIR|\@sysconfdir\@|g' \
-   -e 's|LOCALSTATEDIR|\@localstatedir\@|g' \
-   < $@-t1 > $@-t2 && \
-   rm -f $@-t1 && \
-   mv $@-t2 $@
-
-%.8.in: %.pod
-   $(AM_V_GEN)$(POD2MAN) --section=8 $< $@-t1 && \
-   if grep 'POD ERROR' $@-t1; then rm $@-t1; exit 1; fi && \
-   sed \
-   -e 's|SYSCONFDIR|\@sysconfdir\@|g' \
-   -e 's|LOCALSTATEDIR|\@localstatedir\@|g' \
-   < $@-t1 > $@-t2 && \
-   rm -f $@-t1 && \
-   mv $@-t2 $@
-
-%.1: %.1.in $(top_srcdir)/configure.ac
-   $(AM_V_GEN)sed \
-   -e 's|[@]sysconfdir[@]|$(sysconfdir)|g' \
-   -e 's|[@]localstatedir[@]|$(localstatedir)|g' \
-   < $< > $@-t && \
-   mv $@-t $@
-
-%.8: %.8.in $(top_srcdir)/configure.ac
-   $(AM_V_GEN)sed \
-   -e 's|[@]sysconfdir[@]|$(sysconfdir)|g' \
-   -e 's|[@]localstatedir[@]|$(localstatedir)|g' \
-   < $< > $@-t && \
-   mv $@-t $@
-
 install-data-local: install-systemd install-nss \
install-bash-completion
 
@@ -573,8 +529,5 @@ clean-local:
 
 CLEANFILES += $(bin_SCRIPTS)
 CLEANFILES += *.gcov .libs/*.gcda .libs/*.gcno *.gcno *.gcda *.i *.s
-CLEANFILES += $(man1_MANS) $(man8_MANS)
 
 DISTCLEANFILES += $(BUILT_SOURCES)
-
-MAINTAINERCLEANFILES += $(MANINFILES)
-- 
2.23.0

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

Re: [libvirt] [PATCH 1/3] qemu: fix crash due to freeing an uninitialised pointer

[Pavel Mores]

On Fri, Dec 06, 2019 at 02:49:26PM +0100, Michal Privoznik wrote:
> On 12/6/19 10:11 AM, Pavel Mores wrote:
> > The bug was fairly though not 100% reproducible, presumably due to the fact
> > that some of the time, 'safename' might turn out NULL by chance, in which 
> > case
> > freeing it is OK.
> > 
> > Signed-off-by: Pavel Mores 
> > ---
> >   src/qemu/qemu_monitor_text.c | 2 --
> >   1 file changed, 2 deletions(-)
> 
> D'oh! I've came around this bug in the morning when playing with libvirt-php
> too and merged patch around the same time as you posted this fix.
> 
> https://www.redhat.com/archives/libvir-list/2019-December/msg00341.html
> 
> Sorry,

No problem,  what's important is that it's fixed. :-)

pvl

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

[libvirt] [PATCH 08/17] docs: convert virtlogd man page from pod to rst

[Daniel P . Berrangé]

This was a semi-automated conversion. First it was run through pod2rst,
and then it was manually editted to use a rst structure that matches
expectations of rst2man.

Signed-off-by: Daniel P. Berrangé 
---
 docs/Makefile.am|   2 +
 docs/manpages/index.rst |   1 +
 docs/manpages/virtlogd.rst  | 179 
 src/logging/Makefile.inc.am |  13 ---
 src/logging/virtlogd.pod| 165 -
 5 files changed, 182 insertions(+), 178 deletions(-)
 create mode 100644 docs/manpages/virtlogd.rst
 delete mode 100644 src/logging/virtlogd.pod

diff --git a/docs/Makefile.am b/docs/Makefile.am
index 6615915efb..9fa706f0a9 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -215,11 +215,13 @@ if WITH_LIBVIRTD
 manpages8_rst += \
   manpages/libvirtd.rst \
   manpages/virtlockd.rst \
+  manpages/virtlogd.rst \
   $(NULL)
 else ! WITH_LIBVIRTD
 manpages_rst += \
   manpages/libvirtd.rst \
   manpages/virtlockd.rst \
+  manpages/virtlogd.rst \
   $(NULL)
 endif ! WITH_LIBVIRTD
 manpages_rst_html_in = \
diff --git a/docs/manpages/index.rst b/docs/manpages/index.rst
index dc88cef198..8544fb2501 100644
--- a/docs/manpages/index.rst
+++ b/docs/manpages/index.rst
@@ -7,3 +7,4 @@ Daemons
 
 * `libvirtd(8) `__ - libvirt management daemon
 * `virtlockd(8) `__ - libvirt lock management daemon
+* `virtlogd(8) `__ - libvirt log management daemon
diff --git a/docs/manpages/virtlogd.rst b/docs/manpages/virtlogd.rst
new file mode 100644
index 00..7f4c5d7e7f
--- /dev/null
+++ b/docs/manpages/virtlogd.rst
@@ -0,0 +1,179 @@
+
+virtlogd
+
+
+-
+libvirt log management daemon
+-
+
+:Manual section: 8
+:Manual group: Virtualization Support
+
+.. contents::
+
+SYNOPSIS
+
+
+``virtlogd`` [*OPTION*]...
+
+
+DESCRIPTION
+===
+
+The ``virtlogd`` program is a server side daemon component of the libvirt
+virtualization management system that is used to manage logs from virtual
+machine consoles.
+
+This daemon is not used directly by libvirt client applications, rather it
+is called on their behalf by ``libvirtd``. By maintaining the logs in a
+standalone daemon, the main libvirtd daemon can be restarted without risk
+of losing logs. The ``virtlogd`` daemon has the ability to re-exec()
+itself upon receiving SIGUSR1, to allow live upgrades without downtime.
+
+The ``virtlogd`` daemon listens for requests on a local Unix domain socket.
+
+
+OPTIONS
+===
+
+``-h``, ``--help``
+
+Display command line help usage then exit.
+
+``-d``, ``--daemon``
+
+Run as a daemon and write PID file.
+
+``-f``, ``--config`` *FILE*
+
+Use this configuration file, overriding the default value.
+
+``-t``, ``--timeout`` *SECONDS*
+
+Automatically shutdown after *SECONDS* have elapsed with
+no active console log.
+
+``-p``, ``--pid-file`` *FILE*
+
+Use this name for the PID file, overriding the default value.
+
+``-v``, ``--verbose``
+
+Enable output of verbose messages.
+
+``-V``, ``--version``
+
+Display version information then exit.
+
+
+SIGNALS
+===
+
+On receipt of ``SIGUSR1``, ``virtlogd`` will re-exec() its binary, while
+maintaining all current logs and clients. This allows for live
+upgrades of the ``virtlogd`` service.
+
+
+FILES
+=
+
+When run as *root*
+--
+
+* ``SYSCONFDIR/libvirt/virtlogd.conf``
+
+The default configuration file used by ``virtlogd``, unless overridden on the
+command line using the ``-f``  | ``--config`` option.
+
+* ``RUNSTATEDIR/libvirt/virtlogd-sock``
+
+The sockets ``virtlogd`` will use.
+
+* ``RUNSTATEDIR/virtlogd.pid``
+
+The PID file to use, unless overridden by the ``-p`` | ``--pid-file`` option.
+
+When run as *non-root*
+--
+
+* ``$XDG_CONFIG_HOME/libvirt/virtlogd.conf``
+
+The default configuration file used by ``virtlogd``, unless overridden on the
+command line using the ``-f`` | ``--config`` option.
+
+* ``$XDG_RUNTIME_DIR/libvirt/virtlogd-sock``
+
+The socket ``virtlogd`` will use.
+
+* ``$XDG_RUNTIME_DIR/libvirt/virtlogd.pid``
+
+The PID file to use, unless overridden by the ``-p`` | ``--pid-file`` option.
+
+If ``$XDG_CONFIG_HOME`` is not set in your environment, ``virtlogd`` will use
+``$HOME/.config``
+
+If ``$XDG_RUNTIME_DIR`` is not set in your environment, ``virtlogd`` will use
+``$HOME/.cache``
+
+
+EXAMPLES
+
+
+To retrieve the version of ``virtlogd``:
+
+.. code-block:: shell
+
+  # virtlogd --version
+  virtlogd (libvirt) 1.1.1
+
+To start ``virtlogd``, instructing it to daemonize and create a PID file:
+
+.. code-block:: shell
+
+  # virtlogd -d
+  # ls -la RUNSTATEDIR/virtlogd.pid
+  -rw-r--r-- 1 root root 6 Jul  9 02:40 RUNSTATEDIR/virtlogd.pid
+
+
+BUGS
+
+
+Please report all bugs you discover.  This should be done via either:
+
+#. the mailing list
+
+   `https://libvirt.org/contact.html `_
+
+#. the bug tracker
+
+   `https://libvirt.org/bugs.html 

[libvirt] [PATCH 14/17] docs: convert virt-admin man page from pod to rst

[Daniel P . Berrangé]

This was a semi-automated conversion. First it was run through pod2rst,
and then it was manually editted to use a rst structure that matches
expectations of rst2man.

Signed-off-by: Daniel P. Berrangé 
---
 docs/Makefile.am |   1 +
 docs/manpages/index.rst  |   1 +
 docs/manpages/virt-admin.rst | 610 +++
 tools/Makefile.am|   5 +-
 tools/virt-admin.pod | 497 
 5 files changed, 613 insertions(+), 501 deletions(-)
 create mode 100644 docs/manpages/virt-admin.rst
 delete mode 100644 tools/virt-admin.pod

diff --git a/docs/Makefile.am b/docs/Makefile.am
index d63cfa50dc..1f42afedb6 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -203,6 +203,7 @@ manpages_rst = \
 manpages1_rst = \
   manpages/virt-pki-validate.rst \
   manpages/virt-xml-validate.rst \
+  manpages/virt-admin.rst \
   $(NULL)
 manpages7_rst = \
   $(NULL)
diff --git a/docs/manpages/index.rst b/docs/manpages/index.rst
index 891bf17faf..d7e4bcf1d1 100644
--- a/docs/manpages/index.rst
+++ b/docs/manpages/index.rst
@@ -17,3 +17,4 @@ Tools
 * `virt-xml-validate(1) `__ - validate libvirt XML 
files against a schema
 * `virt-sanlock-cleanup(8) `__ - remove stale 
sanlock resource lease files
 * `virt-login-shell(1) `__ - tool to execute a shell 
within a container
+* `virt-admin(1) `__ - daemon administration interface
diff --git a/docs/manpages/virt-admin.rst b/docs/manpages/virt-admin.rst
new file mode 100644
index 00..8d28642fe3
--- /dev/null
+++ b/docs/manpages/virt-admin.rst
@@ -0,0 +1,610 @@
+==
+virt-admin
+==
+
+---
+daemon administration interface
+---
+
+:Manual section: 1
+:Manual group: Virtualization Support
+
+.. contents::
+
+SYNOPSIS
+
+
+``virt-admin`` [*OPTION*]... [*COMMAND_STRING*]
+
+``virt-admin`` [*OPTION*]... *COMMAND* [*ARG*]...
+
+
+DESCRIPTION
+===
+
+The ``virt-admin`` program is the main administration interface for modifying
+the libvirt daemon configuration at runtime, changing daemon behaviour as well
+as for monitoring and managing all clients connected to the daemon.
+
+The basic structure of most virt-admin usage is:
+
+.. code-block:: shell
+
+   virt-admin [OPTION]...  [ARG]...
+
+Where *command* is one of the commands listed below. Any *command*
+starting with ``#`` is treated as a comment and silently ignored, all
+other unrecognized *commands* are diagnosed.
+
+The ``virt-admin`` program can be used either to run one *COMMAND* by giving 
the
+command and its arguments on the shell command line, or a *COMMAND_STRING*
+which is a single shell argument consisting of multiple *COMMAND* actions
+and their arguments joined with whitespace and separated by semicolons or
+newlines between commands, where unquoted backslash-newline pairs are
+elided.  Within *COMMAND_STRING*, virt-admin understands the
+same single, double, and backslash escapes as the shell, although you must
+add another layer of shell escaping in creating the single shell argument,
+and any word starting with unquoted *#* begins a comment that ends at newline.
+If no command is given in the command line, ``virt-admin`` will then start a 
minimal
+interpreter waiting for your commands, and the ``quit`` command will then exit
+the program.
+
+The ``virt-admin`` program understands the following *OPTIONS*.
+
+
+``-c``, ``--connect`` *URI*
+
+Connect to the specified *URI*, as if by the ``connect`` command,
+instead of the default connection.
+
+``-d``, ``--debug`` *LEVEL*
+
+Enable debug messages at integer *LEVEL* and above.  *LEVEL* can
+range from 0 to 4 (default).  See the documentation of ``VIRT_ADMIN_DEBUG``
+environment variable below for the description of each *LEVEL*.
+
+``-h``, ``--help``
+
+Ignore all other arguments, and behave as if the ``help`` command were
+given instead.
+
+``-l``, ``--log`` *FILE*
+
+Output logging details to *FILE*.
+
+``-q``, ``--quiet``
+
+Avoid extra informational messages.
+
+``-v``, ``--version[=short]``
+
+Ignore all other arguments, and prints the version of the libvirt library
+virt-admin is coming from
+
+``-V``, ``--version=long``
+
+Ignore all other arguments, and prints the version of the libvirt library
+virt-admin is coming from.
+
+
+NOTES
+=
+
+Running ``virt-admin`` requires root privileges due to the
+communications channels used to talk to the daemon. Consider changing the
+*unix_sock_group* ownership setting to grant access to specific set of users
+or modifying *unix_sock_rw_perms* permissions. Daemon configuration file
+provides more information about setting permissions.
+
+
+GENERIC COMMANDS
+
+
+The following commands are generic.
+
+help
+
+
+.. code-block:: shell
+
+   help [command-or-group]
+
+This lists each of the virt-admin commands.  When used without options, all
+commands are listed, one per line, grouped into related categories,
+displaying the keyword for each gro

[libvirt] [PATCH 09/17] docs: convert virt-host-validate man page from pod to rst

[Daniel P . Berrangé]

This was a semi-automated conversion. First it was run through pod2rst,
and then it was manually editted to use a rst structure that matches
expectations of rst2man.

Signed-off-by: Daniel P. Berrangé 
---
 docs/Makefile.am |  5 ++
 docs/manpages/index.rst  |  5 ++
 docs/manpages/virt-host-validate.rst | 95 
 tools/Makefile.am|  3 -
 tools/virt-host-validate.pod | 68 
 5 files changed, 105 insertions(+), 71 deletions(-)
 create mode 100644 docs/manpages/virt-host-validate.rst
 delete mode 100644 tools/virt-host-validate.pod

diff --git a/docs/Makefile.am b/docs/Makefile.am
index 9fa706f0a9..bedcc04bd4 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -224,6 +224,11 @@ manpages_rst += \
   manpages/virtlogd.rst \
   $(NULL)
 endif ! WITH_LIBVIRTD
+if WITH_HOST_VALIDATE
+  manpages8_rst += manpages/virt-host-validate.rst
+else ! WITH_HOST_VALIDATE
+  manpages_rst += manpages/virt-host-validate.rst
+endif ! WITH_HOST_VALIDATE
 manpages_rst_html_in = \
   $(manpages_rst:%.rst=%.html.in)
 manpages_html = \
diff --git a/docs/manpages/index.rst b/docs/manpages/index.rst
index 8544fb2501..a545960919 100644
--- a/docs/manpages/index.rst
+++ b/docs/manpages/index.rst
@@ -8,3 +8,8 @@ Daemons
 * `libvirtd(8) `__ - libvirt management daemon
 * `virtlockd(8) `__ - libvirt lock management daemon
 * `virtlogd(8) `__ - libvirt log management daemon
+
+Tools
+=
+
+* `virt-host-validate(1) `__ - validate host 
virtualization setup
diff --git a/docs/manpages/virt-host-validate.rst 
b/docs/manpages/virt-host-validate.rst
new file mode 100644
index 00..0738694662
--- /dev/null
+++ b/docs/manpages/virt-host-validate.rst
@@ -0,0 +1,95 @@
+==
+virt-host-validate
+==
+
+--
+validate host virtualization setup
+--
+
+:Manual section: 1
+:Manual group: Virtualization Support
+
+.. contents::
+
+SYNOPSIS
+
+
+``virt-host-validate`` [*OPTIONS*...] [*HV-TYPE*]
+
+
+DESCRIPTION
+===
+
+This tool validates that the host is configured in a suitable
+way to run libvirt hypervisor drivers. If invoked without any
+arguments it will check support for all hypervisor drivers it
+is aware of. Optionally it can be given a particular hypervisor
+type (``qemu``, ``lxc`` or ``bhyve``) to restrict the checks
+to those relevant for that virtualization technology
+
+
+OPTIONS
+===
+
+``-v``, ``--version``
+
+Display the command version
+
+``-h``, ``--help``
+
+Display the command line help
+
+``-q``, ``--quiet``
+
+Don't display details of individual checks being performed.
+Only display output if a check does not pass.
+
+
+EXIT STATUS
+===
+
+Upon successful validation, an exit status of 0 will be set. Upon
+failure a non-zero status will be set.
+
+AUTHOR
+==
+
+Daniel P. Berrangé
+
+
+BUGS
+
+
+Please report all bugs you discover.  This should be done via either:
+
+#. the mailing list
+
+   `https://libvirt.org/contact.html `_
+
+#. the bug tracker
+
+   `https://libvirt.org/bugs.html `_
+
+Alternatively, you may report bugs to your software distributor / vendor.
+
+
+COPYRIGHT
+=
+
+Copyright (C) 2012 by Red Hat, Inc.
+
+
+LICENSE
+===
+
+``virt-host-validate`` is distributed under the terms of the GNU GPL v2+.
+This is free software; see the source for copying conditions. There
+is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
+PURPOSE
+
+
+SEE ALSO
+
+
+virsh(1), virt-pki-validate(1), virt-xml-validate(1),
+`https://libvirt.org/ `_
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 1a541a3984..f4e71d38f2 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -54,7 +54,6 @@ ICON_FILES = \
 
 PODFILES = \
virt-admin.pod \
-   virt-host-validate.pod \
virt-login-shell.pod \
virt-pki-validate.pod \
virt-sanlock-cleanup.pod \
@@ -64,7 +63,6 @@ PODFILES = \
 
 MANINFILES = \
virt-admin.1.in \
-   virt-host-validate.1.in \
virt-login-shell.1.in \
virt-pki-validate.1.in \
virt-sanlock-cleanup.8.in \
@@ -118,7 +116,6 @@ endif WITH_LOGIN_SHELL
 
 if WITH_HOST_VALIDATE
 bin_PROGRAMS += virt-host-validate
-man1_MANS += virt-host-validate.1
 endif WITH_HOST_VALIDATE
 
 virt-xml-validate: virt-xml-validate.in Makefile
diff --git a/tools/virt-host-validate.pod b/tools/virt-host-validate.pod
deleted file mode 100644
index 121bb7ed7a..00
--- a/tools/virt-host-validate.pod
+++ /dev/null
@@ -1,68 +0,0 @@
-=head1 NAME
-
-virt-host-validate - validate host virtualization setup
-
-=head1 SYNOPSIS
-
-B [I...] [I]
-
-=head1 DESCRIPTION
-
-This tool validates that the host is configured in a suitable
-way to run libvirt hypervisor drivers. If invoked without any
-arguments it will check support for all

[libvirt] [PATCH 11/17] docs: convert virt-xml-validate man page from pod to rst

[Daniel P . Berrangé]

This was a semi-automated conversion. First it was run through pod2rst,
and then it was manually editted to use a rst structure that matches
expectations of rst2man.

Signed-off-by: Daniel P. Berrangé 
---
 docs/Makefile.am  |   1 +
 docs/manpages/index.rst   |   1 +
 .../manpages/virt-xml-validate.rst| 107 +++---
 tools/Makefile.am |   3 -
 4 files changed, 68 insertions(+), 44 deletions(-)
 rename tools/virt-xml-validate.pod => docs/manpages/virt-xml-validate.rst (53%)

diff --git a/docs/Makefile.am b/docs/Makefile.am
index a5e85390a4..fa41077381 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -202,6 +202,7 @@ manpages_rst = \
   $(NULL)
 manpages1_rst = \
   manpages/virt-pki-validate.rst \
+  manpages/virt-xml-validate.rst \
   $(NULL)
 manpages7_rst = \
   $(NULL)
diff --git a/docs/manpages/index.rst b/docs/manpages/index.rst
index 0d166c1923..5042071c7d 100644
--- a/docs/manpages/index.rst
+++ b/docs/manpages/index.rst
@@ -14,3 +14,4 @@ Tools
 
 * `virt-host-validate(1) `__ - validate host 
virtualization setup
 * `virt-pki-validate(1) `__ - validate libvirt PKI 
files are configured correctly
+* `virt-xml-validate(1) `__ - validate libvirt XML 
files against a schema
diff --git a/tools/virt-xml-validate.pod b/docs/manpages/virt-xml-validate.rst
similarity index 53%
rename from tools/virt-xml-validate.pod
rename to docs/manpages/virt-xml-validate.rst
index a51a57002a..940eada3a8 100644
--- a/tools/virt-xml-validate.pod
+++ b/docs/manpages/virt-xml-validate.rst
@@ -1,14 +1,27 @@
-=head1 NAME
+=
+virt-xml-validate
+=
 
-virt-xml-validate - validate libvirt XML files against a schema
+---
+validate libvirt XML files against a schema
+---
 
-=head1 SYNOPSIS
+:Manual section: 1
+:Manual group: Virtualization Support
 
-B I [I]
+.. contents::
 
-B I
+SYNOPSIS
+
 
-=head1 DESCRIPTION
+
+``virt-xml-validate`` *XML-FILE* [*SCHEMA-NAME*]
+
+``virt-xml-validate`` *OPTION*
+
+
+DESCRIPTION
+===
 
 Validates a libvirt XML for compliance with the published schema.
 The first compulsory argument is the path to the XML file to be
@@ -18,98 +31,110 @@ from the name of the root element in the XML document.
 
 Valid schema names currently include
 
-=over 4
-
-=item C
+- ``domainsnapshot``
 
 The schema for the XML format used by domain snapshot configuration
 
-=item C
+- ``domain``
 
 The schema for the XML format used by guest domains configuration
 
-=item C
+- ``network``
 
 The schema for the XML format used by virtual network configuration
 
-=item C
+- ``storagepool``
 
 The schema for the XML format used by storage pool configuration
 
-=item C
+- ``storagevol``
 
 The schema for the XML format used by storage volume descriptions
 
-=item C
+- ``nodedev``
 
 The schema for the XML format used by node device descriptions
 
-=item C
+- ``capability``
 
 The schema for the XML format used to declare driver capabilities
 
-=item C
+- ``nwfilter``
 
 The schema for the XML format used by network traffic filters
 
-=item C
+- ``nwfilterbinding``
 
 The schema for XML format used by network filter bindings.
 
-=item C
+- ``secret``
 
 The schema for the XML format used by secrets descriptions
 
-=item C
+- ``interface``
 
 The schema for the XML format used by physical host interfaces
 
-=back
-
-=head1 OPTIONS
 
-=over
+OPTIONS
+===
 
-=item B<-h, --help>
+``-h``, ``--help``
 
 Display command line help usage then exit.
 
-=item B<-V, --version>
+``-V``, ``--version``
 
 Display version information then exit.
 
-=back
 
-=head1 EXIT STATUS
+EXIT STATUS
+===
 
 Upon successful validation, an exit status of 0 will be set. Upon
 failure a non-zero status will be set.
 
-=head1 AUTHOR
 
-Daniel P.Berrange
+AUTHOR
+==
+
+Daniel P. Berrangé
+
 
-=head1 BUGS
+BUGS
+
 
-Report any bugs discovered to the libvirt community via the
-mailing list L or bug tracker
-L.
-Alternatively report bugs to your software distributor / vendor.
+Please report all bugs you discover.  This should be done via either:
 
-=head1 COPYRIGHT
+#. the mailing list
+
+   `https://libvirt.org/contact.html `_
+
+#. the bug tracker
+
+   `https://libvirt.org/bugs.html `_
+
+Alternatively, you may report bugs to your software distributor / vendor.
+
+
+COPYRIGHT
+=
 
 Copyright (C) 2009-2013 by Red Hat, Inc.
-Copyright (C) 2009 by Daniel P. Berrange
+Copyright (C) 2009 by Daniel P. Berrangé
+
 
-=head1 LICENSE
+LICENSE
+===
 
-virt-xml-validate is distributed under the terms of the GNU GPL v2+.
+``virt-xml-validate`` is distributed under the terms of the GNU GPL v2+.
 This is free software; see the source for copying conditions. There
 is NO warranty; not even for 

[libvirt] [PATCH 16/17] docs: convert virkeycode*/virkeyname* man pages from pod to rst

[Daniel P . Berrangé]

The keycodemap tool is told to generate docs in rst format now
instead of pod.

Signed-off-by: Daniel P. Berrangé 
---
 docs/Makefile.am | 30 
 docs/manpages/index.rst  | 16 +++
 src/util/Makefile.inc.am | 43 +---
 3 files changed, 47 insertions(+), 42 deletions(-)

diff --git a/docs/Makefile.am b/docs/Makefile.am
index fad506539b..e1f8f7646d 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -197,6 +197,10 @@ kbase_html = \
 kbasedir = $(HTML_DIR)/kbase
 kbase_DATA = $(kbase_html)
 
+# Sync with src/util/
+KEYCODES = linux osx atset1 atset2 atset3 xtkbd usb win32 qnum
+KEYNAMES = linux osx win32
+
 manpages_rst = \
   manpages/index.rst \
   $(NULL)
@@ -207,6 +211,8 @@ manpages1_rst = \
   manpages/virsh.rst \
   $(NULL)
 manpages7_rst = \
+  $(KEYCODES:%=manpages/virkeycode-%.rst) \
+  $(KEYNAMES:%=manpages/virkeyname-%.rst) \
   $(NULL)
 manpages8_rst = \
   manpages/virt-sanlock-cleanup.rst \
@@ -269,6 +275,29 @@ man8_MANS = $(manpages8_rst:%.rst=%.8)
   -e 's|RUNSTATEDIR|$(runstatedir)|g' | \
   $(RST2MAN) > $@
 
+manpages/virkeycode-%.rst: $(top_srcdir)/src/keycodemapdb/data/keymaps.csv \
+   $(top_srcdir)/src/keycodemapdb/tools/keymap-gen Makefile.am
+   $(AM_V_GEN)export NAME=`echo $@ | \
+   sed -e 's,manpages/virkeycode-,,' -e 's,\.rst,,'` && \
+   $(MKDIR_P) manpages/ && \
+   $(RUNUTF8) $(PYTHON) 
$(top_srcdir)/src/keycodemapdb/tools/keymap-gen \
+   code-docs \
+   --lang rst \
+   --title "virkeycode-$$NAME" \
+   --subtitle "Key code values for $$NAME" \
+   $(top_srcdir)/src/keycodemapdb/data/keymaps.csv $$NAME > $@
+
+manpages/virkeyname-%.rst: $(top_srcdir)/src/keycodemapdb/data/keymaps.csv \
+   $(top_srcdir)/src/keycodemapdb/tools/keymap-gen Makefile.am
+   $(AM_V_GEN)export NAME=`echo $@ | \
+   sed -e 's,manpages/virkeyname-,,' -e 's,\.rst,,'` && \
+   $(MKDIR_P) manpages/ && \
+   $(RUNUTF8) $(PYTHON) 
$(top_srcdir)/src/keycodemapdb/tools/keymap-gen \
+   name-docs \
+   --lang rst \
+   --title "virkeyname-$$NAME" \
+   --subtitle "Key name values for $$NAME" \
+   $(top_srcdir)/src/keycodemapdb/data/keymaps.csv $$NAME > $@
 
 manpagesdir = $(HTML_DIR)/manpages
 manpages_DATA = $(manpages_html)
@@ -342,6 +371,7 @@ CLEANFILES = \
   $(manpages_html) \
   $(man1_MANS) \
   $(man7_MANS) \
+  $(manpages7_rst) \
   $(man8_MANS) \
   $(api_DATA) \
   $(dot_html_generated_in) \
diff --git a/docs/manpages/index.rst b/docs/manpages/index.rst
index 1041dbf8b4..4945ad59e2 100644
--- a/docs/manpages/index.rst
+++ b/docs/manpages/index.rst
@@ -19,3 +19,19 @@ Tools
 * `virt-login-shell(1) `__ - tool to execute a shell 
within a container
 * `virt-admin(1) `__ - daemon administration interface
 * `virsh(1) `__ - management user interface
+
+Key codes
+=
+
+* `virkeycode-atset1 `__ - atset1 keycodes
+* `virkeycode-atset2 `__ - atset2 keycodes
+* `virkeycode-atset3 `__ - atset3 keycodes
+* `virkeycode-linux `__ - linux keycodes
+* `virkeycode-qnum `__ - qnmum keycodes
+* `virkeycode-osx `__ - osx keycodes
+* `virkeycode-usb `__ - usb keycodes
+* `virkeycode-win32 `__ - win32 keycodes
+* `virkeycode-xtkbd `__ - xtkbd keycodes
+* `virkeyname-linux `__ - keycodes
+* `virkeyname-osx `__ - osx keynames
+* `virkeyname-win32 `__ - win32 keynames
diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am
index ec10e53606..459378b264 100644
--- a/src/util/Makefile.inc.am
+++ b/src/util/Makefile.inc.am
@@ -242,7 +242,7 @@ EXTRA_DIST += \
$(srcdir)/keycodemapdb/tools/keymap-gen \
$(NULL)
 
-
+# Sync with docs/
 KEYCODES = linux osx atset1 atset2 atset3 xtkbd usb win32 qnum
 KEYNAMES = linux osx win32
 
@@ -251,15 +251,8 @@ KEYTABLES = \
$(KEYNAMES:%=util/virkeynametable_%.h) \
$(NULL)
 
-KEYPODS = $(KEYCODES:%=util/virkeycode-%.pod) \
- $(KEYNAMES:%=util/virkeyname-%.pod)
-KEYMANS = $(KEYPODS:%.pod=%.7)
-
-man7_MANS += $(KEYMANS)
-
 BUILT_SOURCES += $(KEYTABLES)
 CLEANFILES += $(KEYTABLES)
-CLEANFILES += $(KEYMANS) $(KEYPODS)
 
 UTIL_IO_HELPER_SOURCES = util/iohelper.c
 
@@ -323,37 +316,3 @@ util/virkeynametable_%.h: 
$(srcdir)/keycodemapdb/data/keymaps.csv \
name-table --lang stdc --varname virKeyNameTable_$$NAME 
\
$(srcdir)/keycodemapdb/data/keymaps.csv $$NAME > $@-tmp 
&& \
mv $@-tmp $@ || rm -f $@-tmp
-
-util/virkeycode-%.pod: $(srcdir)/keycodemapdb/data/keymaps.csv \
-   $(srcdir)/keycodemapdb/tools/keymap-gen Makefile.am
-   $(AM_V_GEN)export NAME=`echo $@ | sed -e 's,util/virkeycode-,,' \
- -e 's,\.pod,,'` && \
-   $(MKDIR_P) util/ && \
-   $(RUNUTF8) $(PYTHON) $(srcdir

[libvirt] [PATCH 07/17] docs: convert virtlockd man page from pod to rst

[Daniel P . Berrangé]

This was a semi-automated conversion. First it was run through pod2rst,
and then it was manually editted to use a rst structure that matches
expectations of rst2man.

Signed-off-by: Daniel P. Berrangé 
---
 docs/Makefile.am|   2 +
 docs/manpages/index.rst |   1 +
 docs/manpages/virtlockd.rst | 177 
 src/locking/Makefile.inc.am |  18 
 src/locking/virtlockd.pod   | 165 -
 5 files changed, 180 insertions(+), 183 deletions(-)
 create mode 100644 docs/manpages/virtlockd.rst
 delete mode 100644 src/locking/virtlockd.pod

diff --git a/docs/Makefile.am b/docs/Makefile.am
index 8205f28788..6615915efb 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -214,10 +214,12 @@ manpages_rst += \
 if WITH_LIBVIRTD
 manpages8_rst += \
   manpages/libvirtd.rst \
+  manpages/virtlockd.rst \
   $(NULL)
 else ! WITH_LIBVIRTD
 manpages_rst += \
   manpages/libvirtd.rst \
+  manpages/virtlockd.rst \
   $(NULL)
 endif ! WITH_LIBVIRTD
 manpages_rst_html_in = \
diff --git a/docs/manpages/index.rst b/docs/manpages/index.rst
index 4467d88ba1..dc88cef198 100644
--- a/docs/manpages/index.rst
+++ b/docs/manpages/index.rst
@@ -6,3 +6,4 @@ Daemons
 ===
 
 * `libvirtd(8) `__ - libvirt management daemon
+* `virtlockd(8) `__ - libvirt lock management daemon
diff --git a/docs/manpages/virtlockd.rst b/docs/manpages/virtlockd.rst
new file mode 100644
index 00..4066934e42
--- /dev/null
+++ b/docs/manpages/virtlockd.rst
@@ -0,0 +1,177 @@
+=
+virtlockd
+=
+
+--
+libvirt lock management daemon
+--
+
+:Manual section: 8
+:Manual group: Virtualization Support
+
+.. contents::
+
+SYNOPSIS
+
+
+``virtlockd``  [*OPTION*]...
+
+
+DESCRIPTION
+===
+
+The ``virtlockd`` program is a server side daemon component of the libvirt
+virtualization management system that is used to manage locks held against
+virtual machine resources, such as their disks.
+
+This daemon is not used directly by libvirt client applications, rather it
+is called on their behalf by ``libvirtd``. By maintaining the locks in a
+standalone daemon, the main libvirtd daemon can be restarted without risk
+of losing locks.  The ``virtlockd`` daemon has the ability to re-exec()
+itself upon receiving SIGUSR1, to allow live upgrades without downtime.
+
+The ``virtlockd`` daemon listens for requests on a local Unix domain socket.
+
+
+OPTIONS
+===
+
+``-h``, ``--help``
+
+Display command line help usage then exit.
+
+``-d``, ``--daemon``
+
+Run as a daemon and write PID file.
+
+``-f``, ``--config`` *FILE*
+
+Use this configuration file, overriding the default value.
+
+``-t``, ``--timeout`` *SECONDS*
+
+Automatically shutdown after *SECONDS* have elapsed with
+no active client or lock.
+
+``-p``, ``--pid-file`` *FILE*
+
+Use this name for the PID file, overriding the default value.
+
+``-v``, ``--verbose``
+
+Enable output of verbose messages.
+
+``-V``, ``--version``
+
+Display version information then exit.
+
+SIGNALS
+===
+
+On receipt of ``SIGUSR1``, ``virtlockd`` will re-exec() its binary, while
+maintaining all current locks and clients. This allows for live
+upgrades of the ``virtlockd`` service.
+
+
+FILES
+=
+
+When run as *root*
+--
+
+* ``SYSCONFDIR/libvirt/virtlockd.conf``
+
+The default configuration file used by ``virtlockd``, unless overridden on the
+command line using the ``-f`` | ``--config`` option.
+
+* ``RUNSTATEDIR/libvirt/virtlockd-sock``
+
+The sockets ``virtlockd`` will use.
+
+* ``RUNSTATEDIR/virtlockd.pid``
+
+The PID file to use, unless overridden by the ``-p`` | ``--pid-file`` option.
+
+
+When run as *non-root*
+--
+
+* ``$XDG_CONFIG_HOME/libvirt/virtlockd.conf``
+
+The default configuration file used by ``virtlockd``, unless overridden on the
+command line using the ``-f`` | ``--config`` option.
+
+* ``$XDG_RUNTIME_DIR/libvirt/virtlockd-sock``
+
+The socket ``virtlockd`` will use.
+
+* ``$XDG_RUNTIME_DIR/libvirt/virtlockd.pid``
+
+The PID file to use, unless overridden by the ``-p`` | ``--pid-file`` option.
+
+If ``$XDG_CONFIG_HOME`` is not set in your environment, ``virtlockd`` will use
+``$HOME/.config``
+
+If ``$XDG_RUNTIME_DIR`` is not set in your environment, ``virtlockd`` will use
+``$HOME/.cache``
+
+EXAMPLES
+
+
+To retrieve the version of ``virtlockd``:
+
+.. code-block:: shell
+
+  # virtlockd --version
+  virtlockd (libvirt) 1.1.1
+
+To start ``virtlockd``, instructing it to daemonize and create a PID file:
+
+.. code-block:: shell
+
+  # virtlockd -d
+  # ls -la RUNSTATEDIR/virtlockd.pid
+  -rw-r--r-- 1 root root 6 Jul  9 02:40 RUNSTATEDIR/virtlockd.pid
+
+BUGS
+
+
+Please report all bugs you discover.  This should be done via either:
+
+#. the mailing list
+
+   `https://libvirt.org/contact.html `_
+
+#. the bug tracker
+
+   `https://libvirt.org/bugs.html 

[libvirt] [PATCH 10/17] docs: convert virt-pki-validate man page from pod to rst

[Daniel P . Berrangé]

This was a semi-automated conversion. First it was run through pod2rst,
and then it was manually editted to use a rst structure that matches
expectations of rst2man.

Signed-off-by: Daniel P. Berrangé 
---
 docs/Makefile.am|  1 +
 docs/manpages/index.rst |  1 +
 docs/manpages/virt-pki-validate.rst | 89 +
 tools/Makefile.am   |  3 -
 tools/virt-pki-validate.pod | 61 
 5 files changed, 91 insertions(+), 64 deletions(-)
 create mode 100644 docs/manpages/virt-pki-validate.rst
 delete mode 100644 tools/virt-pki-validate.pod

diff --git a/docs/Makefile.am b/docs/Makefile.am
index bedcc04bd4..a5e85390a4 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -201,6 +201,7 @@ manpages_rst = \
   manpages/index.rst \
   $(NULL)
 manpages1_rst = \
+  manpages/virt-pki-validate.rst \
   $(NULL)
 manpages7_rst = \
   $(NULL)
diff --git a/docs/manpages/index.rst b/docs/manpages/index.rst
index a545960919..0d166c1923 100644
--- a/docs/manpages/index.rst
+++ b/docs/manpages/index.rst
@@ -13,3 +13,4 @@ Tools
 =
 
 * `virt-host-validate(1) `__ - validate host 
virtualization setup
+* `virt-pki-validate(1) `__ - validate libvirt PKI 
files are configured correctly
diff --git a/docs/manpages/virt-pki-validate.rst 
b/docs/manpages/virt-pki-validate.rst
new file mode 100644
index 00..730c99ec79
--- /dev/null
+++ b/docs/manpages/virt-pki-validate.rst
@@ -0,0 +1,89 @@
+=
+virt-pki-validate
+=
+
+---
+validate libvirt PKI files are configured correctly
+---
+
+:Manual section: 1
+:Manual group: Virtualization Support
+
+.. contents::
+
+SYNOPSIS
+
+
+
+``virt-pki-validate`` [*OPTION*]
+
+
+DESCRIPTION
+===
+
+This tool validates that the necessary PKI files are configured for
+a secure libvirt server or client using the TLS encryption protocol.
+It will report any missing certificate or key files on the host. It
+should be run as root to ensure it can read all the necessary files
+
+
+OPTIONS
+===
+
+``-h``, ``--help``
+
+Display command line help usage then exit.
+
+``-V``, ``--version``
+
+Display version information then exit.
+
+EXIT STATUS
+===
+
+Upon successful validation, an exit status of 0 will be set. Upon
+failure a non-zero status will be set.
+
+
+AUTHOR
+==
+
+Richard Jones
+
+
+BUGS
+
+
+Please report all bugs you discover.  This should be done via either:
+
+#. the mailing list
+
+   `https://libvirt.org/contact.html `_
+
+#. the bug tracker
+
+   `https://libvirt.org/bugs.html `_
+
+Alternatively, you may report bugs to your software distributor / vendor.
+
+
+COPYRIGHT
+=
+
+Copyright (C) 2006-2012 by Red Hat, Inc.
+
+
+LICENSE
+===
+
+``virt-pki-validate`` is distributed under the terms of the GNU GPL v2+.
+This is free software; see the source for copying conditions. There
+is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
+PURPOSE
+
+
+SEE ALSO
+
+
+virsh(1), `online PKI setup instructions `_,
+`https://www.libvirt.org/ `_
diff --git a/tools/Makefile.am b/tools/Makefile.am
index f4e71d38f2..db690670af 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -55,7 +55,6 @@ ICON_FILES = \
 PODFILES = \
virt-admin.pod \
virt-login-shell.pod \
-   virt-pki-validate.pod \
virt-sanlock-cleanup.pod \
virt-xml-validate.pod \
virsh.pod \
@@ -64,7 +63,6 @@ PODFILES = \
 MANINFILES = \
virt-admin.1.in \
virt-login-shell.1.in \
-   virt-pki-validate.1.in \
virt-sanlock-cleanup.8.in \
virt-xml-validate.1.in \
virsh.1.in \
@@ -96,7 +94,6 @@ bin_SCRIPTS = virt-xml-validate virt-pki-validate
 bin_PROGRAMS = virsh virt-admin
 libexec_SCRIPTS = libvirt-guests.sh
 man1_MANS = \
-   virt-pki-validate.1 \
virt-xml-validate.1 \
virsh.1 \
virt-admin.1
diff --git a/tools/virt-pki-validate.pod b/tools/virt-pki-validate.pod
deleted file mode 100644
index 5d44973fdc..00
--- a/tools/virt-pki-validate.pod
+++ /dev/null
@@ -1,61 +0,0 @@
-=head1 NAME
-
-virt-pki-validate - validate libvirt PKI files are configured correctly
-
-=head1 SYNOPSIS
-
-B [I]
-
-=head1 DESCRIPTION
-
-This tool validates that the necessary PKI files are configured for
-a secure libvirt server or client using the TLS encryption protocol.
-It will report any missing certificate or key files on the host. It
-should be run as root to ensure it can read all the necessary files
-
-=head1 OPTIONS
-
-=over
-
-=item B<-h, --help>
-
-Display command line help usage then exit.
-
-=item B<-V, --version>
-
-Display version information then exit.
-
-=back
-
-=head1 EXIT STATUS
-
-Upon 

[libvirt] [PATCH 13/17] docs: convert virt-login-shell man page from pod to rst

[Daniel P . Berrangé]

This was a semi-automated conversion. First it was run through pod2rst,
and then it was manually editted to use a rst structure that matches
expectations of rst2man.

Signed-off-by: Daniel P. Berrangé 
---
 docs/Makefile.am   |   5 +
 docs/manpages/index.rst|   1 +
 docs/manpages/virt-login-shell.rst | 144 +
 tools/Makefile.am  |   3 -
 tools/virt-login-shell.pod | 112 --
 5 files changed, 150 insertions(+), 115 deletions(-)
 create mode 100644 docs/manpages/virt-login-shell.rst
 delete mode 100644 tools/virt-login-shell.pod

diff --git a/docs/Makefile.am b/docs/Makefile.am
index b652d361b3..d63cfa50dc 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -232,6 +232,11 @@ if WITH_HOST_VALIDATE
 else ! WITH_HOST_VALIDATE
   manpages_rst += manpages/virt-host-validate.rst
 endif ! WITH_HOST_VALIDATE
+if WITH_LOGIN_SHELL
+  manpages1_rst += manpages/virt-login-shell.rst
+else ! WITH_LOGIN_SHELL
+  manpages_rst += manpages/virt-login-shell.rst
+endif ! WITH_LOGIN_SHELL
 manpages_rst_html_in = \
   $(manpages_rst:%.rst=%.html.in)
 manpages_html = \
diff --git a/docs/manpages/index.rst b/docs/manpages/index.rst
index 85a9100ce8..891bf17faf 100644
--- a/docs/manpages/index.rst
+++ b/docs/manpages/index.rst
@@ -16,3 +16,4 @@ Tools
 * `virt-pki-validate(1) `__ - validate libvirt PKI 
files are configured correctly
 * `virt-xml-validate(1) `__ - validate libvirt XML 
files against a schema
 * `virt-sanlock-cleanup(8) `__ - remove stale 
sanlock resource lease files
+* `virt-login-shell(1) `__ - tool to execute a shell 
within a container
diff --git a/docs/manpages/virt-login-shell.rst 
b/docs/manpages/virt-login-shell.rst
new file mode 100644
index 00..4716493f87
--- /dev/null
+++ b/docs/manpages/virt-login-shell.rst
@@ -0,0 +1,144 @@
+
+virt-login-shell
+
+
+--
+tool to execute a shell within a container
+--
+
+:Manual section: 1
+:Manual group: Virtualization Support
+
+.. contents::
+
+SYNOPSIS
+
+
+``virt-login-shell`` [*OPTION*]
+
+
+DESCRIPTION
+===
+
+The ``virt-login-shell`` program is a setuid shell that is used to join
+an LXC container that matches the user's name.  If the container is not
+running, ``virt-login-shell`` will attempt to start the container.
+``virt-login-shell`` is not allowed to be run by root.  Normal users will get
+added to a container that matches their username, if it exists, and they are
+configured in ``/etc/libvirt/virt-login-shell.conf``.
+
+The basic structure of most ``virt-login-shell`` usage is:
+
+.. code-block:: shell
+
+   virt-login-shell
+
+
+OPTIONS
+===
+
+``-c CMD``
+
+Instruct the shell to run CMD instead of presenting an
+interactive shell prompt.
+
+``-h``, ``--help``
+
+Display command line help usage then exit.
+
+``-V``, ``--version``
+
+Display version information then exit.
+
+
+CONFIG
+==
+
+By default, ``virt-login-shell`` will execute the ``/bin/sh`` program for
+the user. You can modify this behaviour by defining the shell variable in
+``/etc/libvirt/virt-login-shell.conf``. e.g.
+
+.. code-block::
+
+   shell = [ "/bin/bash" ]
+
+
+If the ``auto_shell`` config option is set then it will attempt to 
automatically
+detect the shell from ``/etc/password`` inside the container. This should only
+be done if the container has a separate ``/etc`` directory from the host,
+otherwise it will end up recursively invoking ``virt-login-shell``. e.g.
+
+.. code-block::
+
+   auto_shell = 1
+
+
+By default no users are allowed to use virt-login-shell, if you want to allow
+certain users to use virt-login-shell, you need to modify the allowed_users
+variable in /etc/libvirt/virt-login-shell.conf. e.g.
+
+.. code-block::
+
+   allowed_users = [ "tom", "dick", "harry" ]
+
+
+EXIT STATUS
+===
+
+``virt-login-shell`` normally returns the exit status of the command it
+executed. If the command was killed by a signal, but that signal is not
+fatal to virt-login-shell, then it returns the signal number plus 128.
+
+Exit status generated by ``virt-login-shell`` itself:
+
+* ``0`` An option was used to learn more about this binary.
+
+* ``125`` Generic error before attempting execution of the configured shell; 
for example, if libvirtd is not running.
+
+* ``126`` The configured shell exists but could not be executed.
+
+* ``127`` The configured shell could not be found.
+
+
+BUGS
+
+
+Please report all bugs you discover.  This should be done via either:
+
+#. the mailing list
+
+   `https://libvirt.org/contact.html `_
+
+#. the bug tracker
+
+   `https://libvirt.org/bugs.html `_
+
+Alternatively, you may report bugs to your software distributor / vendor.
+
+
+AUTHOR
+==
+
+Daniel Walsh
+
+
+COPYRIGHT
+=
+
+Copyright (C) 2013-2014 Red Hat, Inc., and 

[libvirt] [PATCH 04/17] docs: introduce rst2man as a mandatory tool for building docs

[Daniel P . Berrangé]

The rst2man tool is provided by python docutils, and as the name
suggests, it converts RST documents into man pages.

The intention is that our current POD docs will be converted to
RST format, allowing one more use of Perl to be eliminated from
libvirt.

The manual pages will now all be kept in the docs/manpages/ directory,
which enables us to include the man pages in the published website.
This is good for people searching for libvirt man pages online as it
makes it more likely google will send them to the libvirt.org instead
of some random third party man page site with outdated content.

Signed-off-by: Daniel P. Berrangé 
---
 docs/Makefile.am | 60 
 docs/docs.html.in|  3 ++
 docs/manpages/index.rst  |  3 ++
 m4/virt-external-programs.m4 |  5 +++
 4 files changed, 71 insertions(+)
 create mode 100644 docs/manpages/index.rst

diff --git a/docs/Makefile.am b/docs/Makefile.am
index 5f5dce28eb..34fea87805 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -197,6 +197,54 @@ kbase_html = \
 kbasedir = $(HTML_DIR)/kbase
 kbase_DATA = $(kbase_html)
 
+manpages_rst = \
+  manpages/index.rst \
+  $(NULL)
+manpages1_rst = \
+  $(NULL)
+manpages7_rst = \
+  $(NULL)
+manpages8_rst = \
+  $(NULL)
+manpages_rst += \
+  $(manpages1_rst) \
+  $(manpages7_rst) \
+  $(manpages8_rst) \
+  $(NULL)
+manpages_rst_html_in = \
+  $(manpages_rst:%.rst=%.html.in)
+manpages_html = \
+  $(manpages_rst_html_in:%.html.in=%.html)
+
+man1_MANS = $(manpages1_rst:%.rst=%.1)
+man7_MANS = $(manpages7_rst:%.rst=%.7)
+man8_MANS = $(manpages8_rst:%.rst=%.8)
+
+%.1: %.rst
+   $(AM_V_GEN)$(MKDIR_P) `dirname $@` && \
+  grep -v '^\.\. contents::' < $< | \
+  sed -e 's|SYSCONFDIR|$(sysconfdir)|g' \
+  -e 's|RUNSTATEDIR|$(runstatedir)|g' | \
+  $(RST2MAN) > $@
+
+%.7: %.rst
+   $(AM_V_GEN)$(MKDIR_P) `dirname $@` && \
+  grep -v '^\.\. contents::' < $< | \
+  sed -e 's|SYSCONFDIR|$(sysconfdir)|g' \
+  -e 's|RUNSTATEDIR|$(runstatedir)|g' | \
+  $(RST2MAN) > $@
+
+%.8: %.rst
+   $(AM_V_GEN)$(MKDIR_P) `dirname $@` && \
+  grep -v '^\.\. contents::' < $< | \
+  sed -e 's|SYSCONFDIR|$(sysconfdir)|g' \
+  -e 's|RUNSTATEDIR|$(runstatedir)|g' | \
+  $(RST2MAN) > $@
+
+
+manpagesdir = $(HTML_DIR)/manpages
+manpages_DATA = $(manpages_html)
+
 # Generate hvsupport.html and news.html first, since they take one extra step.
 dot_html_generated_in = \
   hvsupport.html.in \
@@ -244,6 +292,7 @@ EXTRA_DIST= \
   $(javascript) $(logofiles) \
   $(internals_html_in) $(internals_rst) $(fonts) \
   $(kbase_html_in) $(kbase_rst) \
+  $(manpages_rst) \
   aclperms.htmlinc \
   hvsupport.pl \
   $(schema_DATA)
@@ -262,6 +311,10 @@ CLEANFILES = \
   $(apilxchtml) \
   $(internals_html) \
   $(kbase_html) \
+  $(manpages_html) \
+  $(man1_MANS) \
+  $(man7_MANS) \
+  $(man8_MANS) \
   $(api_DATA) \
   $(dot_html_generated_in) \
   aclperms.htmlinc
@@ -298,6 +351,13 @@ EXTRA_DIST += \
 %.png: %.fig
convert -rotate 90 $< $@
 
+manpages/%.html.in: manpages/%.rst
+   $(AM_V_GEN)$(MKDIR_P) `dirname $@` && \
+grep -v '^:Manual ' < $< | \
+ sed -e 's|SYSCONFDIR|$(sysconfdir)|g' \
+-e 's|RUNSTATEDIR|$(runstatedir)|g' | \
+ $(RST2HTML) > $@
+
 %.html.in: %.rst
$(AM_V_GEN)$(MKDIR_P) `dirname $@` && \
  $(RST2HTML) $< > $@
diff --git a/docs/docs.html.in b/docs/docs.html.in
index f441769617..fbf35234d4 100644
--- a/docs/docs.html.in
+++ b/docs/docs.html.in
@@ -9,6 +9,9 @@
 Applications
 Applications known to use libvirt
 
+Manual pages
+Manual pages for libvirt tools / daemons
+
 Windows
 Downloads for Windows
 
diff --git a/docs/manpages/index.rst b/docs/manpages/index.rst
new file mode 100644
index 00..11c72135e8
--- /dev/null
+++ b/docs/manpages/index.rst
@@ -0,0 +1,3 @@
+
+Libvirt Manual Pages
+
diff --git a/m4/virt-external-programs.m4 b/m4/virt-external-programs.m4
index ed634a4c73..9b128382fa 100644
--- a/m4/virt-external-programs.m4
+++ b/m4/virt-external-programs.m4
@@ -38,6 +38,11 @@ AC_DEFUN([LIBVIRT_CHECK_EXTERNAL_PROGRAMS], [
   then
 AC_MSG_ERROR("rst2html is required to build libvirt")
   fi
+  AC_PATH_PROGS([RST2MAN], [rst2man rst2man.py rst2man-3], [])
+  if test -z "$RST2MAN"
+  then
+AC_MSG_ERROR("rst2man is required to build libvirt")
+  fi
   AC_PATH_PROG([AUGPARSE], [augparse], [/usr/bin/augparse])
   AC_PROG_MKDIR_P
   AC_PROG_LN_S
-- 
2.23.0

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

[libvirt] [PATCH 12/17] docs: convert virt-sanlock-cleanup man page from pod to rst

[Daniel P . Berrangé]

This was a semi-automated conversion. First it was run through pod2rst,
and then it was manually editted to use a rst structure that matches
expectations of rst2man.

Signed-off-by: Daniel P. Berrangé 
---
 docs/Makefile.am   |  1 +
 docs/manpages/index.rst|  1 +
 docs/manpages/virt-sanlock-cleanup.rst | 79 ++
 tools/Makefile.am  |  3 -
 tools/virt-sanlock-cleanup.pod | 49 
 5 files changed, 81 insertions(+), 52 deletions(-)
 create mode 100644 docs/manpages/virt-sanlock-cleanup.rst
 delete mode 100644 tools/virt-sanlock-cleanup.pod

diff --git a/docs/Makefile.am b/docs/Makefile.am
index fa41077381..b652d361b3 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -207,6 +207,7 @@ manpages1_rst = \
 manpages7_rst = \
   $(NULL)
 manpages8_rst = \
+  manpages/virt-sanlock-cleanup.rst \
   $(NULL)
 manpages_rst += \
   $(manpages1_rst) \
diff --git a/docs/manpages/index.rst b/docs/manpages/index.rst
index 5042071c7d..85a9100ce8 100644
--- a/docs/manpages/index.rst
+++ b/docs/manpages/index.rst
@@ -15,3 +15,4 @@ Tools
 * `virt-host-validate(1) `__ - validate host 
virtualization setup
 * `virt-pki-validate(1) `__ - validate libvirt PKI 
files are configured correctly
 * `virt-xml-validate(1) `__ - validate libvirt XML 
files against a schema
+* `virt-sanlock-cleanup(8) `__ - remove stale 
sanlock resource lease files
diff --git a/docs/manpages/virt-sanlock-cleanup.rst 
b/docs/manpages/virt-sanlock-cleanup.rst
new file mode 100644
index 00..ac49fa4191
--- /dev/null
+++ b/docs/manpages/virt-sanlock-cleanup.rst
@@ -0,0 +1,79 @@
+
+virt-sanlock-cleanup
+
+
+-
+remove stale sanlock resource lease files
+-
+
+:Manual section: 1
+:Manual group: Virtualization Support
+
+.. contents::
+
+SYNOPSIS
+
+
+``virt-sanlock-cleanup``
+
+
+DESCRIPTION
+===
+
+This tool removes any resource lease files created by the sanlock
+lock manager plugin. The resource lease files only need to exist
+on disks when a guest using the resource is active. This script
+reclaims the disk space used by resources which are not currently
+active.
+
+
+EXIT STATUS
+===
+
+Upon successful processing of leases cleanup, an exit status
+of 0 will be set. Upon fatal error a non-zero status will
+be set.
+
+
+AUTHOR
+==
+
+Daniel P. Berrangé
+
+
+BUGS
+
+
+Please report all bugs you discover.  This should be done via either:
+
+#. the mailing list
+
+   `https://libvirt.org/contact.html `_
+
+#. the bug tracker
+
+   `https://libvirt.org/bugs.html `_
+
+Alternatively, you may report bugs to your software distributor / vendor.
+
+
+COPYRIGHT
+=
+
+Copyright (C) 2011, 2013 Red Hat, Inc.
+
+
+LICENSE
+===
+
+``virt-sanlock-cleanup`` is distributed under the terms of the GNU GPL v2+.
+This is free software; see the source for copying conditions. There
+is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
+PURPOSE
+
+
+SEE ALSO
+
+
+virsh(1), `online instructions `_,
+`https://libvirt.org/ `_
diff --git a/tools/Makefile.am b/tools/Makefile.am
index ca4dcfc9f6..01bba235a4 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -55,14 +55,12 @@ ICON_FILES = \
 PODFILES = \
virt-admin.pod \
virt-login-shell.pod \
-   virt-sanlock-cleanup.pod \
virsh.pod \
$(NULL)
 
 MANINFILES = \
virt-admin.1.in \
virt-login-shell.1.in \
-   virt-sanlock-cleanup.8.in \
virsh.1.in \
$(NULL)
 
@@ -97,7 +95,6 @@ man1_MANS = \
 
 if WITH_SANLOCK
 sbin_SCRIPTS = virt-sanlock-cleanup
-man8_MANS = virt-sanlock-cleanup.8
 DISTCLEANFILES += virt-sanlock-cleanup
 endif WITH_SANLOCK
 
diff --git a/tools/virt-sanlock-cleanup.pod b/tools/virt-sanlock-cleanup.pod
deleted file mode 100644
index 41012566a1..00
--- a/tools/virt-sanlock-cleanup.pod
+++ /dev/null
@@ -1,49 +0,0 @@
-=head1 NAME
-
-virt-sanlock-cleanup - remove stale sanlock resource lease files
-
-=head1 SYNOPSIS
-
-B
-
-=head1 DESCRIPTION
-
-This tool removes any resource lease files created by the sanlock
-lock manager plugin. The resource lease files only need to exist
-on disks when a guest using the resource is active. This script
-reclaims the disk space used by resources which are not currently
-active.
-
-=head1 EXIT STATUS
-
-Upon successful processing of leases cleanup, an exit status
-of 0 will be set. Upon fatal error a non-zero status will
-be set.
-
-=head1 AUTHOR
-
-Daniel Berrange
-
-=head1 BUGS
-
-Report any bugs discovered to the libvirt community via the
-mailing list L or bug tracker
-L.
-Alternatively report bugs to your software distributor / ven

[libvirt] [PATCH 06/17] docs: convert libvirtd man page from pod to rst

[Daniel P . Berrangé]

This was a semi-automated conversion. First it was run through pod2rst,
and then it was manually editted to use a rst structure that matches
expectations of rst2man.

Signed-off-by: Daniel P. Berrangé 
---
 docs/Makefile.am   |   9 ++
 docs/manpages/index.rst|   5 +
 docs/manpages/libvirtd.rst | 259 +
 src/remote/Makefile.inc.am |  15 ---
 src/remote/libvirtd.pod| 235 -
 5 files changed, 273 insertions(+), 250 deletions(-)
 create mode 100644 docs/manpages/libvirtd.rst
 delete mode 100644 src/remote/libvirtd.pod

diff --git a/docs/Makefile.am b/docs/Makefile.am
index 34fea87805..8205f28788 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -211,6 +211,15 @@ manpages_rst += \
   $(manpages7_rst) \
   $(manpages8_rst) \
   $(NULL)
+if WITH_LIBVIRTD
+manpages8_rst += \
+  manpages/libvirtd.rst \
+  $(NULL)
+else ! WITH_LIBVIRTD
+manpages_rst += \
+  manpages/libvirtd.rst \
+  $(NULL)
+endif ! WITH_LIBVIRTD
 manpages_rst_html_in = \
   $(manpages_rst:%.rst=%.html.in)
 manpages_html = \
diff --git a/docs/manpages/index.rst b/docs/manpages/index.rst
index 11c72135e8..4467d88ba1 100644
--- a/docs/manpages/index.rst
+++ b/docs/manpages/index.rst
@@ -1,3 +1,8 @@
 
 Libvirt Manual Pages
 
+
+Daemons
+===
+
+* `libvirtd(8) `__ - libvirt management daemon
diff --git a/docs/manpages/libvirtd.rst b/docs/manpages/libvirtd.rst
new file mode 100644
index 00..b7489a57fc
--- /dev/null
+++ b/docs/manpages/libvirtd.rst
@@ -0,0 +1,259 @@
+
+libvirtd
+
+
+-
+libvirt management daemon
+-
+
+:Manual section: 8
+:Manual group: Virtualization Support
+
+.. contents::
+
+SYNOPSIS
+
+
+``libvirtd`` [*OPTION*]...
+
+
+DESCRIPTION
+===
+
+The ``libvirtd`` program is the server side daemon component of the libvirt
+virtualization management system.
+
+This daemon runs on host servers and performs required management tasks for
+virtualized guests.  This includes activities such as starting, stopping
+and migrating guests between host servers, configuring and manipulating
+networking, and managing storage for use by guests.
+
+The libvirt client libraries and utilities connect to this daemon to issue
+tasks and collect information about the configuration and resources of the host
+system and guests.
+
+By default, the libvirtd daemon listens for requests on a local Unix domain
+socket.  Using the ``-l`` | ``--listen`` command line option, the libvirtd 
daemon
+can be instructed to additionally listen on a TCP/IP socket.  The TCP/IP socket
+to use is defined in the libvirtd configuration file.
+
+Restarting libvirtd does not impact running guests.  Guests continue to operate
+and will be picked up automatically if their XML configuration has been
+defined.  Any guests whose XML configuration has not been defined will be lost
+from the configuration.
+
+
+SYSTEM SOCKET ACTIVATION
+
+
+The ``libvirtd`` daemon is capable of starting in two modes.
+
+In the traditional mode, it will create and listen on UNIX sockets itself.
+If the ``--listen`` parameter is given, it will also listen on TCP/IP 
socket(s),
+according to the ``listen_tcp`` and ``listen_tls`` options in
+``/etc/libvirt/libvirtd.conf``
+
+In socket activation mode, it will rely on systemd to create and listen
+on the UNIX, and optionally TCP/IP, sockets and pass them as pre-opened
+file descriptors. In this mode, it is not permitted to pass the ``--listen``
+parameter, and most of the socket related config options in
+``/etc/libvirt/libvirtd.conf`` will no longer have any effect. To enable
+TCP or TLS sockets use either
+
+::
+
+   $ systemctl start libvirtd-tls.socket
+
+Or
+
+::
+
+   $ systemctl start libvirtd-tcp.socket
+
+Socket activation mode is generally the default when running on a host
+OS that uses systemd. To revert to the traditional mode, all the socket
+unit files must be masked:
+
+::
+
+   $ systemctl mask libvirtd.socket libvirtd-ro.socket \
+  libvirtd-admin.socket libvirtd-tls.socket libvirtd-tcp.socket
+
+
+OPTIONS
+===
+
+``-h``, ``--help``
+
+Display command line help usage then exit.
+
+``-d``, ``--daemon``
+
+Run as a daemon & write PID file.
+
+``-f``, ``--config *FILE*``
+
+Use this configuration file, overriding the default value.
+
+``-l``, ``--listen``
+
+Listen for TCP/IP connections. This should not be set if using systemd
+socket activation. Instead activate the libvirtd-tls.socket or
+libvirtd-tcp.socket unit files.
+
+``-p``, ``--pid-file *FILE*``
+
+Use this name for the PID file, overriding the default value.
+
+``-t``, ``--timeout *SECONDS*``
+
+Exit after timeout period (in seconds), provided there are neither any client
+connections nor any running domains.
+
+``-v``, ``--verbose``
+
+Enable output of verbose messages.
+
+``--version``
+
+Display version information then exit.
+
+
+SIGNALS
+==

[libvirt] [PATCH 01/17] src: update keycodemapdb submodule

[Daniel P . Berrangé]

Pull in changes which support use of RST for docs output format
instead of POD.

The generator tool has changed its command line arg handling
so all args must be after the command name. The docs title and
subtitle must be specified separately too.

Signed-off-by: Daniel P. Berrangé 
---
 src/keycodemapdb |  2 +-
 src/util/Makefile.inc.am | 16 
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/keycodemapdb b/src/keycodemapdb
index 6280c94f30..317d3eeb96 16
--- a/src/keycodemapdb
+++ b/src/keycodemapdb
@@ -1 +1 @@
-Subproject commit 6280c94f306df6a20bbc100ba15a5a81af0366e6
+Subproject commit 317d3eeb963a515e15a63fa356d8ebcda7041a51
diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am
index 0855f152fd..ec10e53606 100644
--- a/src/util/Makefile.inc.am
+++ b/src/util/Makefile.inc.am
@@ -310,7 +310,7 @@ util/virkeycodetable_%.h: 
$(srcdir)/keycodemapdb/data/keymaps.csv \
  -e 's,\.h,,'` && \
$(MKDIR_P) util/ && \
$(RUNUTF8) $(PYTHON) $(srcdir)/keycodemapdb/tools/keymap-gen \
-   --lang stdc --varname virKeyCodeTable_$$NAME code-table 
\
+   code-table --lang stdc --varname virKeyCodeTable_$$NAME 
\
$(srcdir)/keycodemapdb/data/keymaps.csv $$NAME > $@-tmp 
&& \
mv $@-tmp $@ || rm -f $@-tmp
 
@@ -320,7 +320,7 @@ util/virkeynametable_%.h: 
$(srcdir)/keycodemapdb/data/keymaps.csv \
  -e 's,\.h,,'` && \
$(MKDIR_P) util/ && \
$(RUNUTF8) $(PYTHON) $(srcdir)/keycodemapdb/tools/keymap-gen \
-   --lang stdc --varname virKeyNameTable_$$NAME name-table 
\
+   name-table --lang stdc --varname virKeyNameTable_$$NAME 
\
$(srcdir)/keycodemapdb/data/keymaps.csv $$NAME > $@-tmp 
&& \
mv $@-tmp $@ || rm -f $@-tmp
 
@@ -330,9 +330,9 @@ util/virkeycode-%.pod: 
$(srcdir)/keycodemapdb/data/keymaps.csv \
  -e 's,\.pod,,'` && \
$(MKDIR_P) util/ && \
$(RUNUTF8) $(PYTHON) $(srcdir)/keycodemapdb/tools/keymap-gen \
-   --lang pod \
-   --varname "virkeycode-$$NAME - Key code values for $$NAME" \
-   code-docs \
+   code-docs --lang pod \
+   --title "virkeycode-$$NAME" \
+   --subtitle "Key code values for $$NAME" \
$(srcdir)/keycodemapdb/data/keymaps.csv $$NAME > \
$@-tmp && mv $@-tmp $@ || rm $@-tmp
 
@@ -342,9 +342,9 @@ util/virkeyname-%.pod: 
$(srcdir)/keycodemapdb/data/keymaps.csv \
  -e 's,\.pod,,'` && \
$(MKDIR_P) util/ && \
$(RUNUTF8) $(PYTHON) $(srcdir)/keycodemapdb/tools/keymap-gen \
-   --lang pod \
-   --varname "virkeyname-$$NAME - Key name values for $$NAME" \
-   name-docs \
+   name-docs --lang pod \
+   --title "virkeyname-$$NAME" \
+   --subtitle "Key name values for $$NAME" \
$(srcdir)/keycodemapdb/data/keymaps.csv $$NAME > \
$@-tmp && mv $@-tmp $@ || rm $@-tmp
 
-- 
2.23.0

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

[libvirt] [PATCH 03/17] rpm: use python3-docutils as the direct dep

[Daniel P . Berrangé]

We no longer support python2, so using a file based dep for rst2html
is not required. We do still have to do special casing for RHEL-7
though as the RPM is annoyingly different.

Signed-off-by: Daniel P. Berrangé 
---
 libvirt.spec.in | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 4999df525e..8e05d3d0ea 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -266,8 +266,11 @@ BuildRequires: gettext-devel
 BuildRequires: libtool
 %endif
 BuildRequires: /usr/bin/pod2man
-# Replace with python3-docutils when we drop py2 support
-BuildRequires: /usr/bin/rst2html
+%if 0%{?rhel} == 7
+BuildRequires: python36-docutils
+%else
+BuildRequires: python3-docutils
+%endif
 BuildRequires: gcc
 BuildRequires: git
 %if 0%{?fedora} || 0%{?rhel} > 7
-- 
2.23.0

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

[libvirt] [PATCH 05/17] docs: describe the basic RST structure for a man page

[Daniel P . Berrangé]

Signed-off-by: Daniel P. Berrangé 
---
 docs/styleguide.rst | 86 +
 1 file changed, 86 insertions(+)

diff --git a/docs/styleguide.rst b/docs/styleguide.rst
index 71f29320cb..2998e90963 100644
--- a/docs/styleguide.rst
+++ b/docs/styleguide.rst
@@ -64,3 +64,89 @@ which allows for 6 levels of headings
 
Heading 6
^
+
+Manual pages
+
+
+RST documents created as manual pages must have the following structure
+
+::
+
+  ===
+  ::PROGRAM::
+  ===
+
+  ---
+  ...line line description...
+  ---
+
+  :Manual section: 8
+  :Manual group: Virtualization Support
+
+  .. contents::
+
+  SYNOPSIS
+  
+
+  ``::PROGRAM::`` [*OPTION*]...
+
+  DESCRIPTION
+  ===
+
+  ...describe the tool / program ...
+
+  OPTIONS
+  ===
+
+  ``-h``, ``--help``
+
+  Display command line help usage then exit.
+
+  ...and other args
+
+  FILES
+  =
+
+  ...any files used that the user needs to know about. eg config
+  files in particular...
+
+  AUTHORS
+  ===
+
+  Please refer to the AUTHORS file distributed with libvirt.
+
+  BUGS
+  
+
+  Please report all bugs you discover.  This should be done via either:
+
+  #. the mailing list
+
+   `https://libvirt.org/contact.html `_
+
+  #. the bug tracker
+
+   `https://libvirt.org/bugs.html `_
+
+  Alternatively, you may report bugs to your software distributor / vendor.
+
+
+  COPYRIGHT
+  =
+
+  Copyright (C) ::DATE:: ::ORIGINAL AUTHOR::, and the authors listed in the
+  libvirt AUTHORS file.
+
+  LICENSE
+  ===
+
+  ``::PROGRAM::`` is distributed under the terms of the GNU LGPL v2.1+.
+  This is free software; see the source for copying conditions. There
+  is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
+  PURPOSE
+
+  SEE ALSO
+  
+
+  ...other man page links
+  `https://www.libvirt.org/ `_
-- 
2.23.0

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

[libvirt] [PATCH 02/17] rpm: move pod2man & rst2html deps outside the autotools conditional

[Daniel P . Berrangé]

The generated man pages were previously bundled in the dist, so pod2man
was inside the autotools conditional. We no longer bundle any generated
files in the dist though, so pod2man must always be present.

rst2html then mistakenly just followed what pod2man did.

Signed-off-by: Daniel P. Berrangé 
---
 libvirt.spec.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libvirt.spec.in b/libvirt.spec.in
index c431f0dfb5..4999df525e 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -264,10 +264,10 @@ BuildRequires: autoconf
 BuildRequires: automake
 BuildRequires: gettext-devel
 BuildRequires: libtool
+%endif
 BuildRequires: /usr/bin/pod2man
 # Replace with python3-docutils when we drop py2 support
 BuildRequires: /usr/bin/rst2html
-%endif
 BuildRequires: gcc
 BuildRequires: git
 %if 0%{?fedora} || 0%{?rhel} > 7
-- 
2.23.0

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

[libvirt] [PATCH 00/17] docs: remove all use of POD for man pages in favour of RST

[Daniel P . Berrangé]

As part of the goal to eliminate Perl from libvirt, this gets rid of the
use of POD format for man pages. There's nothing especially bad about
POD as a markup language compared to other lightweight markup languages
like RST or Markdown. It hasn't found widespread usage outside of the
Perl world though, and so switching from POD to RST brings a language
which likely has more familiarity to contributors.

This also nicely aligns with our use of RST of web pages, and indeed
in this series things are setup so that all the man pages get published
on the main libvirt.org website. Over time this will hopefuly draw
search engines traffic to libvirt.org instead of random 3rd party
websites hosting various out of date copies of the man pages.

Reviewing the individual RST files is likely a very unpleasant task,
especially the enourmous virsh man page. Most of the conversion work
was automated with pod2rst, followed by lots of editting to cleanup
its output. virsh had some further automated processing done to create
headers for each command.

It is probably more useful to review the rendered man page output
and/or websites to see that it looks sane when read.

Daniel P. Berrangé (17):
  src: update keycodemapdb submodule
  rpm: move pod2man & rst2html deps outside the autotools conditional
  rpm: use python3-docutils as the direct dep
  docs: introduce rst2man as a mandatory tool for building docs
  docs: describe the basic RST structure for a man page
  docs: convert libvirtd man page from pod to rst
  docs: convert virtlockd man page from pod to rst
  docs: convert virtlogd man page from pod to rst
  docs: convert virt-host-validate man page from pod to rst
  docs: convert virt-pki-validate man page from pod to rst
  docs: convert virt-xml-validate man page from pod to rst
  docs: convert virt-sanlock-cleanup man page from pod to rst
  docs: convert virt-login-shell man page from pod to rst
  docs: convert virt-admin man page from pod to rst
  docs: convert virsh man page from pod to rst
  docs: convert virkeycode*/virkeyname* man pages from pod to rst
  docs: remove build recipes related to pod2man usage

 docs/Makefile.am  |  118 +
 docs/docs.html.in |3 +
 docs/manpages/index.rst   |   37 +
 docs/manpages/libvirtd.rst|  259 +
 docs/manpages/virsh.rst   | 7096 +
 docs/manpages/virt-admin.rst  |  610 ++
 docs/manpages/virt-host-validate.rst  |   95 +
 docs/manpages/virt-login-shell.rst|  144 +
 docs/manpages/virt-pki-validate.rst   |   89 +
 docs/manpages/virt-sanlock-cleanup.rst|   79 +
 .../manpages/virt-xml-validate.rst|  107 +-
 docs/manpages/virtlockd.rst   |  177 +
 docs/manpages/virtlogd.rst|  179 +
 docs/styleguide.rst   |   86 +
 libvirt.spec.in   |7 +-
 m4/virt-external-programs.m4  |5 +
 src/Makefile.am   |   18 -
 src/keycodemapdb  |2 +-
 src/locking/Makefile.inc.am   |   18 -
 src/locking/virtlockd.pod |  165 -
 src/logging/Makefile.inc.am   |   13 -
 src/logging/virtlogd.pod  |  165 -
 src/remote/Makefile.inc.am|   15 -
 src/remote/libvirtd.pod   |  235 -
 src/util/Makefile.inc.am  |   47 +-
 tools/Makefile.am |   68 -
 tools/virsh.pod   | 5526 -
 tools/virt-admin.pod  |  497 --
 tools/virt-host-validate.pod  |   68 -
 tools/virt-login-shell.pod|  112 -
 tools/virt-pki-validate.pod   |   61 -
 tools/virt-sanlock-cleanup.pod|   49 -
 32 files changed, 9052 insertions(+), 7098 deletions(-)
 create mode 100644 docs/manpages/index.rst
 create mode 100644 docs/manpages/libvirtd.rst
 create mode 100644 docs/manpages/virsh.rst
 create mode 100644 docs/manpages/virt-admin.rst
 create mode 100644 docs/manpages/virt-host-validate.rst
 create mode 100644 docs/manpages/virt-login-shell.rst
 create mode 100644 docs/manpages/virt-pki-validate.rst
 create mode 100644 docs/manpages/virt-sanlock-cleanup.rst
 rename tools/virt-xml-validate.pod => docs/manpages/virt-xml-validate.rst (53%)
 create mode 100644 docs/manpages/virtlockd.rst
 create mode 100644 docs/manpages/virtlogd.rst
 delete mode 100644 src/locking/virtlockd.pod
 delete mode 100644 src/logging/virtlogd.pod
 delete mode 100644 src/remote/libvirtd.pod
 delete mode 100644 tools/virsh.pod
 delete mode 100644 tools/virt-admin.pod
 delete mode 100644 tools/virt-host-validate.pod
 delete mode 100644 tools/virt-login-shell.pod
 delete mode 100644 tools/virt-pki-validate.pod
 delete mode 100644 tools/virt-sanlock

Re: [libvirt] [PATCH v2 20/25] qemu: Implement backup job APIs and qemu handling

[Vladimir Sementsov-Ogievskiy]

05.12.2019 21:13, Eric Blake wrote:
> [adding some qemu visibility]
> 
> On 12/5/19 7:34 AM, Peter Krempa wrote:
> 
 +    if (!(mergebitmapsstore = virJSONValueCopy(mergebitmaps)))
 +    return -1;
 +
 +    if (qemuMonitorTransactionBitmapAdd(actions,
 +    dd->domdisk->src->nodeformat,
 +    dd->incrementalBitmap,
 +    false,
 +    true) < 0)
 +    return -1;
 +
 +    if (qemuMonitorTransactionBitmapMerge(actions,
 +  dd->domdisk->src->nodeformat,
 +  dd->incrementalBitmap,
 +  &mergebitmaps) < 0)
 +    return -1;
 +
 +    if (qemuMonitorTransactionBitmapAdd(actions,
 +    dd->store->nodeformat,
 +    dd->incrementalBitmap,
 +    false,
 +    true) < 0)
 +    return -1;
>>>
>>> Why do we need two of these calls?
>>> /me reads on
>>>
 +
 +    if (qemuMonitorTransactionBitmapMerge(actions,
 +  dd->store->nodeformat,
 +  dd->incrementalBitmap,
 +  &mergebitmapsstore) < 0)
 +    return -1;
>>>
>>> okay, it looks like you are trying to merge the same bitmap into two
>>> different merge commands, which will all be part of the same transaction.  I
>>> guess it would be helpful to see a trace of the transaction in action to see
>>> if we can simplify it (using 2 instead of 4 qemuMonitor* commands).
>>
>> This is required because the backup blockjob requires the bitmap to be
>> present on the source ('device') image of the backup. The same also
>> applies for the image exported by NBD. The catch is that we expose the
>> scratch file via NBD which is actually the target image of the backup.
>> This means that in case of a pull backup we need two instances of the
>> bitmap so both the block job and the NBD server can use them. Arguably
>> there's a possible simplification here for push-mode backups where the
>> target bitmap doesn't make sense.
> 
> The backup job requires the bitmap to be on the source, but the qemu NBD 
> export code only requires the bitmap to be locatable somewhere on the qemu 
> NBD server requires the bitmap to be discoverable from anywhere on the chain, 
> and since the temporary target of the block job has the source image as its 
> backing file, that should be the case.  That is:
> 
> base <- active <- temp
>    |
>      bitmap0
> 
> looking up [active, bitmap0] or [temp, bitmap0] should both succeed; we need 
> the former for the blockjob, and the latter for the NBD export.
> 
> If the NBD export _can't_ find bitmap0 through the backing chain, that may be 
> a symptom of the problem that Max has been trying to fix (his upcoming v7 
> "deal with filters" is hinted at here, but will not be in 4.2):
> https://lists.gnu.org/archive/html/qemu-devel/2019-11/msg04520.html

these problems will hit if some filters are in use, like throttling, 
copy-on-read, etc. They use file child, which breaks backing chains. But normal 
backing chains should work well.

> 
> In my original implementation, I did not need a duplicated bitmap in the temp 
> file.  But that was pre-blockdev.  Are we really hitting filter limitations 
> in qemu where the use of blockdev is preventing [temp, bitmap0] from finding 
> the bitmap across the backing chain?  If so, we should fix that in qemu - but 
> we're so late for 4.2, that I guess libvirt will have to work around it.
> 



-- 
Best regards,
Vladimir

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

Re: [libvirt] [PATCH for-4.2?] block/qcow2-bitmap: fix crash bug in qcow2_co_remove_persistent_dirty_bitmap

[Eric Blake]

[adding in Peter Maydell, as there is now potential talk of other 
4.2-worthy patches]


On 12/6/19 4:18 AM, Vladimir Sementsov-Ogievskiy wrote:

05.12.2019 23:16, John Snow wrote:



On 12/5/19 3:09 PM, Eric Blake wrote:

On 12/5/19 1:30 PM, Vladimir Sementsov-Ogievskiy wrote:

Here is double bug:

First, return error but not set errp. This may lead to:
qmp block-dirty-bitmap-remove may report success when actually failed

block-dirty-bitmap-remove used in a transaction will crash, as
qmp_transaction will think that it returned success and will cal


call


block_dirty_bitmap_remove_commit which will crash, as state->bitmap is
NULL

Second (like in anecdote), this case is not an error at all. As it is
documented in the comment above bdrv_co_remove_persistent_dirty_bitmap
definition, absence of bitmap is not an error, and similar case handled
at start of qcow2_co_remove_persistent_dirty_bitmap, it returns 0 when
there is no bitmaps at all..


double .



But when there are some bitmaps, but not the requested one, it return
error with errp unset.

Fix that.

Fixes: b56a1e31759b750
Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

Hi all!

Ohm, suddenly we faced this bug. It's a regression in 4.2. I'm very
sorry for introducing it, and it sad that it's found so late..

Personally, I think that this one worth rc5, as it makes new bitmap
interfaces unusable. But the decision is yours.

Last minute edit: hmm, actually, transaction action introduced in
4.2, so crash is not a regression, only broken block-dirty-bitmap-remove
command is a regression... Maybe it's OK for stable.


Libvirt REALLY wants to use transaction bitmap management (and require
qemu 4.2) for its incremental backup patches that Peter is almost ready
to merge in.  I'm trying to ascertain:

When exactly can this bug hit?  Can you give a sequence of QMP commands
that will trigger it for easy reproduction?  Are there workarounds (such
as checking that a bitmap exists prior to attempting to remove it)?  If
it does NOT get fixed in rc5, how will libvirt be able to probe whether
the fix is in place?



It looks like:

- You need to have at least one bitmap
- You need to use transactional remove
- you need to misspell the bitmap name
- The remove will fail (return -EINVAL) but doesn't set errp
- The caller chokes on incomplete information, state->bitmap is NULL



No, that would be too simple, the thing is worse. Absolutele correct removing 
is broken, without any misspelling

Bug triggers when we are removing persistent bitmap that is not stored yet in 
the image AND at least one (another) bitmap already stored in the image. So, 
something like:

1. create persistent bitmap A
2. shutdown vm  (bitmap A is synced)
3. start vm
4. create persistent bitmap B
5. remove bitmap B - it fails (and crashes if in transaction)



Hmm, workaround..

I'm afraid that the only thing is not remove persistent bitmaps, which were 
never synced to the image. So, instead the sequence above, we need


1. create persistent bitmap A
2. shutdown vm
3. start vm
4. create persistent bitmap B
5. remember, that we want to remove bitmap B after vm shutdown
...
   some other operations
...
6. vm shutdown
7. start vm in stopped mode, and remove all bitmaps marked for removing
8. stop vm

But, I think that in real circumstances, vm shutdown is rare thing...


This is sounding a bit more serious. As I said earlier, it shouldn't 
delay 4.2 on its own, but if the fix is obvious (and other than 
comments, it is a single change from 'ret = -EINVAL' to 'ret = 0' which 
fixes a definite reproducible crash), I think it rises to the level of 
acceptable.


I've been so worried about the question of which release, that I don't 
know if I've previously offered:

Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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

Re: [libvirt] [PATCH for-4.2?] block/qcow2-bitmap: fix crash bug in qcow2_co_remove_persistent_dirty_bitmap

[Vladimir Sementsov-Ogievskiy]

05.12.2019 23:09, Eric Blake wrote:
> On 12/5/19 1:30 PM, Vladimir Sementsov-Ogievskiy wrote:
>> Here is double bug:
>>
>> First, return error but not set errp. This may lead to:
>> qmp block-dirty-bitmap-remove may report success when actually failed
>>
>> block-dirty-bitmap-remove used in a transaction will crash, as
>> qmp_transaction will think that it returned success and will cal
> 
> call
> 
>> block_dirty_bitmap_remove_commit which will crash, as state->bitmap is
>> NULL
>>
>> Second (like in anecdote), this case is not an error at all. As it is
>> documented in the comment above bdrv_co_remove_persistent_dirty_bitmap
>> definition, absence of bitmap is not an error, and similar case handled
>> at start of qcow2_co_remove_persistent_dirty_bitmap, it returns 0 when
>> there is no bitmaps at all..
> 
> double .
> 
>>
>> But when there are some bitmaps, but not the requested one, it return
>> error with errp unset.
>>
>> Fix that.
>>
>> Fixes: b56a1e31759b750
>> Signed-off-by: Vladimir Sementsov-Ogievskiy 
>> ---
>>
>> Hi all!
>>
>> Ohm, suddenly we faced this bug. It's a regression in 4.2. I'm very
>> sorry for introducing it, and it sad that it's found so late..
>>
>> Personally, I think that this one worth rc5, as it makes new bitmap
>> interfaces unusable. But the decision is yours.
>>
>> Last minute edit: hmm, actually, transaction action introduced in
>> 4.2, so crash is not a regression, only broken block-dirty-bitmap-remove
>> command is a regression... Maybe it's OK for stable.
> 
> Libvirt REALLY wants to use transaction bitmap management (and require qemu 
> 4.2) for its incremental backup patches that Peter is almost ready to merge 
> in.  I'm trying to ascertain:

Side question:

Is libvirt prepared to inconsistent bitmaps?

For example, migration with enabled dirty-bitmaps capability will fail if there 
are inconsistent bitmaps.
Also, incremental backup may be affected (you see, that you have your bitmap, 
try to do something with it,
aiming to start next incremental backup, but bitmap is inconsistent, and 
operation fails)..

In fact, we in Virtuozzo now faced with these broken migration and backup 
because of inconsistent bitmaps
(which were ignored in past), and we have to apply a temporary fix like

--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -986,8 +986,8 @@ bool qcow2_load_dirty_bitmaps(BlockDriverState *bs, Error 
**errp)
  QSIMPLEQ_FOREACH(bm, bm_list, entry) {
  BdrvDirtyBitmap *bitmap;

-if ((bm->flags & BME_FLAG_IN_USE) &&
-bdrv_find_dirty_bitmap(bs, bm->name))
+if ((bm->flags & BME_FLAG_IN_USE) /* &&
+bdrv_find_dirty_bitmap(bs, bm->name) */)
  {
  /*
   * We already have corresponding BdrvDirtyBitmap, and bitmap in 
the
@@ -1000,6 +1000,13 @@ bool qcow2_load_dirty_bitmaps(BlockDriverState *bs, 
Error **errp)
   * should not load it on migration target, as we already have this
   * bitmap, being migrated.
   */
+
+/*
+ * VZ7.12:
+ *
+ * We don't load inconsistent bitmaps at all, as they block
+ * migration. So the second condition is commented out.
+ */
  continue;
  }

And correct way is to handle inconsistent bitmaps in libvirt somehow.


Note, that the source of inconsistent bitmaps are crashes, hard resets, etc, 
when Qemu is unable to store bitmaps correctly.


> 
> When exactly can this bug hit?  Can you give a sequence of QMP commands that 
> will trigger it for easy reproduction?  Are there workarounds (such as 
> checking that a bitmap exists prior to attempting to remove it)?  If it does 
> NOT get fixed in rc5, how will libvirt be able to probe whether the fix is in 
> place?
> 
>>
>>   block/qcow2-bitmap.c | 9 ++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
>> index 8abaf632fc..c6c8ebbe89 100644
>> --- a/block/qcow2-bitmap.c
>> +++ b/block/qcow2-bitmap.c
>> @@ -1469,8 +1469,10 @@ int coroutine_fn 
>> qcow2_co_remove_persistent_dirty_bitmap(BlockDriverState *bs,
>>   Qcow2BitmapList *bm_list;
>>   if (s->nb_bitmaps == 0) {
>> -    /* Absence of the bitmap is not an error: see explanation above
>> - * bdrv_remove_persistent_dirty_bitmap() definition. */
>> +    /*
>> + * Absence of the bitmap is not an error: see explanation above
>> + * bdrv_co_remove_persistent_dirty_bitmap() definition.
>> + */
>>   return 0;
>>   }
>> @@ -1485,7 +1487,8 @@ int coroutine_fn 
>> qcow2_co_remove_persistent_dirty_bitmap(BlockDriverState *bs,
>>   bm = find_bitmap_by_name(bm_list, name);
>>   if (bm == NULL) {
>> -    ret = -EINVAL;
>> +    /* Absence of the bitmap is not an error, see above. */
>> +    ret = 0;
>>   goto out;
>>   }
>>
> 


-- 
Best regards,
Vladimir


--
libvir-list mailing list

Re: [libvirt] [PATCH 1/3] qemu: fix crash due to freeing an uninitialised pointer

[Michal Privoznik]

On 12/6/19 10:11 AM, Pavel Mores wrote:

The bug was fairly though not 100% reproducible, presumably due to the fact
that some of the time, 'safename' might turn out NULL by chance, in which case
freeing it is OK.

Signed-off-by: Pavel Mores 
---
  src/qemu/qemu_monitor_text.c | 2 --
  1 file changed, 2 deletions(-)


D'oh! I've came around this bug in the morning when playing with 
libvirt-php too and merged patch around the same time as you posted this 
fix.


https://www.redhat.com/archives/libvir-list/2019-December/msg00341.html

Sorry,
Michal

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

Re: [libvirt] [PATCH] qemu: remove nested branching to enhance readability

[Michal Privoznik]

On 12/6/19 10:45 AM, Pavel Mores wrote:

This is a follow-up to patch series posted in

https://www.redhat.com/archives/libvir-list/2019-November/msg01180.html

It implements a suggestion made by Cole in

https://www.redhat.com/archives/libvir-list/2019-November/msg01207.html

and discussed in follow-up messages as there were no objections to the
change.

The aim is to make the code more readable by replacing nested branching
with a flat structure.

Signed-off-by: Pavel Mores 
---
  src/qemu/qemu_domain.c | 19 ---
  1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d1596a28ca..5e4eede3c2 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7914,19 +7914,16 @@ static int
  qemuDomainDefaultVideoDevice(const virDomainDef *def,
virQEMUCapsPtr qemuCaps)
  {
-if (ARCH_IS_PPC64(def->os.arch)) {
+if (ARCH_IS_PPC64(def->os.arch))
  return VIR_DOMAIN_VIDEO_TYPE_VGA;
-} else if (qemuDomainIsARMVirt(def) ||
-   qemuDomainIsRISCVVirt(def) ||
-   ARCH_IS_S390(def->os.arch)) {
+if (qemuDomainIsARMVirt(def) || qemuDomainIsRISCVVirt(def) ||
+ARCH_IS_S390(def->os.arch))


Actually, I like it more if these are on separate lines.


  return VIR_DOMAIN_VIDEO_TYPE_VIRTIO;
-} else {
-if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA))
-return VIR_DOMAIN_VIDEO_TYPE_CIRRUS;
-if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA))
-return VIR_DOMAIN_VIDEO_TYPE_VGA;
-return VIR_DOMAIN_VIDEO_TYPE_DEFAULT;
-}
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA))
+return VIR_DOMAIN_VIDEO_TYPE_CIRRUS;
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA))
+return VIR_DOMAIN_VIDEO_TYPE_VGA;
+return VIR_DOMAIN_VIDEO_TYPE_DEFAULT;
  }
  
  



Reviewed-by: Michal Privoznik 

and pushed.

Michal

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

Re: [libvirt] [PATCH 0/4] Introduce VIR_MIGRATE_PARAM_TLS_DESTINATION migration param

[Pavel Hrdina]

On Tue, Dec 03, 2019 at 04:33:54PM +0100, Jiri Denemark wrote:
> Normally the TLS certificate from the destination host must match the
> host's name for TLS verification to succeed. When the certificate does
> not match the destination hostname and the expected cetificate's
> hostname is known, this parameter can be used to pass this expected
> hostname when starting the migration.
> 
> Jiri Denemark (4):
>   qemu: Add support for setting string migration params
>   Introduce VIR_MIGRATE_PARAM_TLS_DESTINATION migration param
>   qemu: Implement VIR_MIGRATE_PARAM_TLS_DESTINATION
>   virsh: Add --tls-destination option for migrate command

Reviewed-by: Pavel Hrdina 


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 0/9] Introduce mediate ops in vfio-pci

[Yan Zhao]

On Fri, Dec 06, 2019 at 05:40:02PM +0800, Jason Wang wrote:
> 
> On 2019/12/6 下午4:22, Yan Zhao wrote:
> > On Thu, Dec 05, 2019 at 09:05:54PM +0800, Jason Wang wrote:
> >> On 2019/12/5 下午4:51, Yan Zhao wrote:
> >>> On Thu, Dec 05, 2019 at 02:33:19PM +0800, Jason Wang wrote:
>  Hi:
> 
>  On 2019/12/5 上午11:24, Yan Zhao wrote:
> > For SRIOV devices, VFs are passthroughed into guest directly without 
> > host
> > driver mediation. However, when VMs migrating with passthroughed VFs,
> > dynamic host mediation is required to  (1) get device states, (2) get
> > dirty pages. Since device states as well as other critical information
> > required for dirty page tracking for VFs are usually retrieved from PFs,
> > it is handy to provide an extension in PF driver to centralizingly 
> > control
> > VFs' migration.
> >
> > Therefore, in order to realize (1) passthrough VFs at normal time, (2)
> > dynamically trap VFs' bars for dirty page tracking and
>  A silly question, what's the reason for doing this, is this a must for 
>  dirty
>  page tracking?
> 
> >>> For performance consideration. VFs' bars should be passthoughed at
> >>> normal time and only enter into trap state on need.
> >>
> >> Right, but how does this matter for the case of dirty page tracking?
> >>
> > Take NIC as an example, to trap its VF dirty pages, software way is
> > required to trap every write of ring tail that resides in BAR0.
> 
> 
> Interesting, but it looks like we need:
> - decode the instruction
> - mediate all access to BAR0
> All of which seems a great burden for the VF driver. I wonder whether or 
> not doing interrupt relay and tracking head is better in this case.
>
hi Jason

not familiar with the way you mentioned. could you elaborate more?
> 
> >   There's
> > still no IOMMU Dirty bit available.
> > (3) centralizing
> > VF critical states retrieving and VF controls into one driver, we 
> > propose
> > to introduce mediate ops on top of current vfio-pci device driver.
> >
> >
> >   _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
> > __   register mediate ops|  ___ ___|
> > |  |<---| VF|   |   |
> > | vfio-pci |  | |  mediate  |   | PF driver |   |
> > |__|--->|   driver  |   |___|
> > |open(pdev)  |  ---  | |
> > ||
> > ||_ _ _ _ _ _ _ _ _ _ _ _|_ _ _ _ _|
> >\|/  \|/
> > --- 
> > |VF   | |PF|
> > --- 
> >
> >
> > VF mediate driver could be a standalone driver that does not bind to
> > any devices (as in demo code in patches 5-6) or it could be a built-in
> > extension of PF driver (as in patches 7-9) .
> >
> > Rather than directly bind to VF, VF mediate driver register a mediate
> > ops into vfio-pci in driver init. vfio-pci maintains a list of such
> > mediate ops.
> > (Note that: VF mediate driver can register mediate ops into vfio-pci
> > before vfio-pci binding to any devices. And VF mediate driver can
> > support mediating multiple devices.)
> >
> > When opening a device (e.g. a VF), vfio-pci goes through the mediate ops
> > list and calls each vfio_pci_mediate_ops->open() with pdev of the 
> > opening
> > device as a parameter.
> > VF mediate driver should return success or failure depending on it
> > supports the pdev or not.
> > E.g. VF mediate driver would compare its supported VF devfn with the
> > devfn of the passed-in pdev.
> > Once vfio-pci finds a successful vfio_pci_mediate_ops->open(), it will
> > stop querying other mediate ops and bind the opening device with this
> > mediate ops using the returned mediate handle.
> >
> > Further vfio-pci ops (VFIO_DEVICE_GET_REGION_INFO ioctl, rw, mmap) on 
> > the
> > VF will be intercepted into VF mediate driver as
> > vfio_pci_mediate_ops->get_region_info(),
> > vfio_pci_mediate_ops->rw,
> > vfio_pci_mediate_ops->mmap, and get customized.
> > For vfio_pci_mediate_ops->rw and vfio_pci_mediate_ops->mmap, they will
> > further return 'pt' to indicate whether vfio-pci should further
> > passthrough data to hw.
> >
> > when vfio-pci closes the VF, it calls its 
> > vfio_pci_mediate_ops->release()
> > with a mediate handle as parameter.
> >
> > The mediate handle returned from vfio_pci_mediate_ops->open() lets VF
> > mediate driver be able to differentiat

Re: [libvirt] [PATCH] qemu: remove nested branching to enhance readability

[Daniel Henrique Barboza]

On 12/6/19 6:45 AM, Pavel Mores wrote:

This is a follow-up to patch series posted in

https://www.redhat.com/archives/libvir-list/2019-November/msg01180.html

It implements a suggestion made by Cole in

https://www.redhat.com/archives/libvir-list/2019-November/msg01207.html

and discussed in follow-up messages as there were no objections to the
change.

The aim is to make the code more readable by replacing nested branching
with a flat structure.

Signed-off-by: Pavel Mores 
---



Reviewed-by: Daniel Henrique Barboza 

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

Re: [libvirt] [PATCH 0/3] Remove deprecated pc-0.x machine types and related hacks

[Gerd Hoffmann]

> > $ git-grep pc-0
> > hw/display/vga-pci.c:/* compatibility with pc-0.13 and older */
> > hw/display/vga.c:/* With pc-0.12 and below we map both the PCI BAR and 
> > the fixed VBE region,
> > hw/display/vmware_vga.c:/* compatibility with pc-0.13 and older */
> 
> These are the "rombar" hacks that I've mentioned above. The question is
> whether we want to remove them or whether I should just adjust the comments?

Hmm.  All that cruft ...

vga maps the framebuffer @ 0xe000 with rombar=off. It's an alias of
the pci memory bar.  rombar=off is basically a flag for "really old
firmware" here, vgabios used to have that address hardcoded, a decade
ago.  We fixed that roughly the same timeframe where we switched to
seabios, which in turn allowed us to place the vgabios in the pci rom
bar (instead of copying it to 0xa000 in guest ram).  Which is probably
the reason why we have only one switch for both.

I don't expect anyone actually sets the rombar property for vga devices
(it's more common for NICs, for network boot tweaks), so I guess we can
get away with simply dropping the hacks in vga-pci.c and vmware_vga.c.
The comment in vga.c is not fully correct though, isa-vga needs that
too, so we have to keep vga_init_vbe for the time being ...

cheers,
  Gerd

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

Re: [libvirt] Offline manipulation of Dirty Bitmaps by qemu-img

[Vladimir Sementsov-Ogievskiy]

06.12.2019 1:37, John Snow wrote:
> This has come up in the past, and I believe we discussed this at KVM
> Forum, too:
> 
> There have been requests from oVirt (via Nir Soffer) to add some offline
> bitmap manipulation functionality. In the past, our stance has generally
> been "Use QEMU without an accelerator, and use QMP to manipulate the
> images."
> 
> We like this for a few reasons:
> 
> 1. It keeps bitmap management code tightly centralized
> 2. It allows for the full suite of bitmap manipulations in either
> offline or online mode with one tool
> 3. We wouldn't have to write new code.
> 4. Or design new CLIs and duplicate our existing work.
> 5. Or write even more tests.
> 
> However, tools like oVirt may or may not be fully equipped to launch
> QEMU in this context, and there is always a desire for qemu-img to be
> able to "do more", so existing management suites could extend
> functionality more easily.

I think, all guys, who don't want to use Qemu directly for image manipulations,
should hope for Kevin's "[RFC PATCH 00/18] Add qemu-storage-daemon", which is
the correct solution of their problem. Still, I'm not one of these guys.

> 
> (Or so I am imagining.)
> 
> I am still leaning heavily against adding any more CLI commands or
> options to qemu-img right now. Even if we do add some of the fundamental
> ones like "add" or "remove", it seems only a matter of time before we
> have to add "clear", "merge", etc. Is this just a race to code duplication?
> 
> On the other hand, one of the other suggestions is to have qemu-img
> check --repair optionally delete corrupted bitmaps. I kind of like this
> idea: it's a buyer beware operation that might make management layers
> unhappy, but then again ... repair is always something that could make
> things worse.
> 
> Plus, if you manage to corrupt bitmaps badly enough that they can't even
> be parsed, you might need a heavyweight repair operation.
> 

Improving "check" is a correct thing, because it's done inside qcow2 driver
itself. We just don't have corresponding qmp command or command line option
for Qemu to use this thing (or I missed it).

> Nir, do you think that'd be sufficient for your needs for now, or would
> you still like to see more granular offline management?
> 
> --js
> 


-- 
Best regards,
Vladimir

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

Re: [libvirt] [PATCH 3/3] hw/pci: Remove the "command_serr_enable" property

[Paolo Bonzini]

On 05/12/19 17:06, Thomas Huth wrote:
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index db75c6dfd0..5b6ebd15c6 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -174,9 +174,6 @@ enum {
>  #define QEMU_PCI_CAP_MULTIFUNCTION_BITNR3
>  QEMU_PCI_CAP_MULTIFUNCTION = (1 << QEMU_PCI_CAP_MULTIFUNCTION_BITNR),
>  
> -/* command register SERR bit enabled */
> -#define QEMU_PCI_CAP_SERR_BITNR 4
> -QEMU_PCI_CAP_SERR = (1 << QEMU_PCI_CAP_SERR_BITNR),
>  /* Standard hot plug controller. */
>  #define QEMU_PCI_SHPC_BITNR 5
>  QEMU_PCI_CAP_SHPC = (1 << QEMU_PCI_SHPC_BITNR),

I think it's okay to keep this enum.

Paolo

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

Re: [libvirt] [PATCH for-4.2?] block/qcow2-bitmap: fix crash bug in qcow2_co_remove_persistent_dirty_bitmap

[Vladimir Sementsov-Ogievskiy]

05.12.2019 23:16, John Snow wrote:
> 
> 
> On 12/5/19 3:09 PM, Eric Blake wrote:
>> On 12/5/19 1:30 PM, Vladimir Sementsov-Ogievskiy wrote:
>>> Here is double bug:
>>>
>>> First, return error but not set errp. This may lead to:
>>> qmp block-dirty-bitmap-remove may report success when actually failed
>>>
>>> block-dirty-bitmap-remove used in a transaction will crash, as
>>> qmp_transaction will think that it returned success and will cal
>>
>> call
>>
>>> block_dirty_bitmap_remove_commit which will crash, as state->bitmap is
>>> NULL
>>>
>>> Second (like in anecdote), this case is not an error at all. As it is
>>> documented in the comment above bdrv_co_remove_persistent_dirty_bitmap
>>> definition, absence of bitmap is not an error, and similar case handled
>>> at start of qcow2_co_remove_persistent_dirty_bitmap, it returns 0 when
>>> there is no bitmaps at all..
>>
>> double .
>>
>>>
>>> But when there are some bitmaps, but not the requested one, it return
>>> error with errp unset.
>>>
>>> Fix that.
>>>
>>> Fixes: b56a1e31759b750
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy 
>>> ---
>>>
>>> Hi all!
>>>
>>> Ohm, suddenly we faced this bug. It's a regression in 4.2. I'm very
>>> sorry for introducing it, and it sad that it's found so late..
>>>
>>> Personally, I think that this one worth rc5, as it makes new bitmap
>>> interfaces unusable. But the decision is yours.
>>>
>>> Last minute edit: hmm, actually, transaction action introduced in
>>> 4.2, so crash is not a regression, only broken block-dirty-bitmap-remove
>>> command is a regression... Maybe it's OK for stable.
>>
>> Libvirt REALLY wants to use transaction bitmap management (and require
>> qemu 4.2) for its incremental backup patches that Peter is almost ready
>> to merge in.  I'm trying to ascertain:
>>
>> When exactly can this bug hit?  Can you give a sequence of QMP commands
>> that will trigger it for easy reproduction?  Are there workarounds (such
>> as checking that a bitmap exists prior to attempting to remove it)?  If
>> it does NOT get fixed in rc5, how will libvirt be able to probe whether
>> the fix is in place?
>>
> 
> It looks like:
> 
> - You need to have at least one bitmap
> - You need to use transactional remove
> - you need to misspell the bitmap name
> - The remove will fail (return -EINVAL) but doesn't set errp
> - The caller chokes on incomplete information, state->bitmap is NULL


No, that would be too simple, the thing is worse. Absolutele correct removing 
is broken, without any misspelling

Bug triggers when we are removing persistent bitmap that is not stored yet in 
the image AND at least one (another) bitmap already stored in the image. So, 
something like:

1. create persistent bitmap A
2. shutdown vm  (bitmap A is synced)
3. start vm
4. create persistent bitmap B
5. remove bitmap B - it fails (and crashes if in transaction)



Hmm, workaround..

I'm afraid that the only thing is not remove persistent bitmaps, which were 
never synced to the image. So, instead the sequence above, we need


1. create persistent bitmap A
2. shutdown vm
3. start vm
4. create persistent bitmap B
5. remember, that we want to remove bitmap B after vm shutdown
...
  some other operations
...
6. vm shutdown
7. start vm in stopped mode, and remove all bitmaps marked for removing
8. stop vm

But, I think that in real circumstances, vm shutdown is rare thing...


> 
>>>
>>>    block/qcow2-bitmap.c | 9 ++---
>>>    1 file changed, 6 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
>>> index 8abaf632fc..c6c8ebbe89 100644
>>> --- a/block/qcow2-bitmap.c
>>> +++ b/block/qcow2-bitmap.c
>>> @@ -1469,8 +1469,10 @@ int coroutine_fn
>>> qcow2_co_remove_persistent_dirty_bitmap(BlockDriverState *bs,
>>>    Qcow2BitmapList *bm_list;
>>>      if (s->nb_bitmaps == 0) {
>>> -    /* Absence of the bitmap is not an error: see explanation above
>>> - * bdrv_remove_persistent_dirty_bitmap() definition. */
>>> +    /*
>>> + * Absence of the bitmap is not an error: see explanation above
>>> + * bdrv_co_remove_persistent_dirty_bitmap() definition.
>>> + */
>>>    return 0;
>>>    }
>>>    @@ -1485,7 +1487,8 @@ int coroutine_fn
>>> qcow2_co_remove_persistent_dirty_bitmap(BlockDriverState *bs,
>>>      bm = find_bitmap_by_name(bm_list, name);
>>>    if (bm == NULL) {
>>> -    ret = -EINVAL;
>>> +    /* Absence of the bitmap is not an error, see above. */
>>> +    ret = 0;
>>>    goto out;
>>>    }
>>>   
>>
> 


-- 
Best regards,
Vladimir


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

Re: [libvirt] Offline manipulation of Dirty Bitmaps by qemu-img

[Peter Krempa]

On Thu, Dec 05, 2019 at 17:37:11 -0500, John Snow wrote:
> This has come up in the past, and I believe we discussed this at KVM
> Forum, too:
> 
> There have been requests from oVirt (via Nir Soffer) to add some offline
> bitmap manipulation functionality. In the past, our stance has generally
> been "Use QEMU without an accelerator, and use QMP to manipulate the
> images."

This is a thing I wanted to do for a long time but never had time for.
I'm not sure whether that will change though.

We have a workaround for this tough: you can start the VM with CPUs
stopped:

virsh start --pause $VMNAME 

(That translates to VIR_DOMAIN_START_PAUSED flag for
virDomainCreateWithFlags).

You can then use any libvirt API which requires a running VM including
blockjobs checkpoints etc.

The VM then can be destroyed. Since the CPUs didn't run the guest
visible image content was nott modified.

Alternatively to make this slightly more official we could introduce a
new flag for the VM starting API which will actually start the VM in the
no-machine mode, will interlock certain operations such as resuming of
the execution or migration perhaps  and the official purpose will be to
allow complex blockjobs without starting the actual VM.

Since starting an actual VM will be impossible anyways until such a VM
is gone it might be a sane thing to do here.

> We like this for a few reasons:
> 
> 1. It keeps bitmap management code tightly centralized
> 2. It allows for the full suite of bitmap manipulations in either
> offline or online mode with one tool
> 3. We wouldn't have to write new code.
> 4. Or design new CLIs and duplicate our existing work.
> 5. Or write even more tests.

In libvirt we'd like to use it because qemu-img has no reasonable
progress reporting and we could reuse the code we have for interacting
with the jobs when the VM is running.

> However, tools like oVirt may or may not be fully equipped to launch
> QEMU in this context, and there is always a desire for qemu-img to be
> able to "do more", so existing management suites could extend
> functionality more easily.
>
> (Or so I am imagining.)
> 
> I am still leaning heavily against adding any more CLI commands or
> options to qemu-img right now. Even if we do add some of the fundamental
> ones like "add" or "remove", it seems only a matter of time before we
> have to add "clear", "merge", etc. Is this just a race to code duplication?
> 
> On the other hand, one of the other suggestions is to have qemu-img
> check --repair optionally delete corrupted bitmaps. I kind of like this
> idea: it's a buyer beware operation that might make management layers
> unhappy, but then again ... repair is always something that could make
> things worse.

Well, dealing with corrupted bitmaps will be possible. I plan to expose
in the checkpoint XML whether a ckeckpoint is invalid (if it contains at
least one corrupted bitmap) and the user will have the option to delete
all previous checkpoints including the corrupted one to clear any
problem.

Note that deleting only the corrupted checkpoint will not be possible
until it's the oldest one as we attempt to merge them into the previous
ones. We could alternatively add a flag to skip merging of the invalid
checkpoint.

> Plus, if you manage to corrupt bitmaps badly enough that they can't even
> be parsed, you might need a heavyweight repair operation.
> 
> Nir, do you think that'd be sufficient for your needs for now, or would
> you still like to see more granular offline management?
> 
> --js
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

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

[libvirt] [PATCH] qemu: remove nested branching to enhance readability

[Pavel Mores]

This is a follow-up to patch series posted in

https://www.redhat.com/archives/libvir-list/2019-November/msg01180.html

It implements a suggestion made by Cole in

https://www.redhat.com/archives/libvir-list/2019-November/msg01207.html

and discussed in follow-up messages as there were no objections to the
change.

The aim is to make the code more readable by replacing nested branching
with a flat structure.

Signed-off-by: Pavel Mores 
---
 src/qemu/qemu_domain.c | 19 ---
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d1596a28ca..5e4eede3c2 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7914,19 +7914,16 @@ static int
 qemuDomainDefaultVideoDevice(const virDomainDef *def,
   virQEMUCapsPtr qemuCaps)
 {
-if (ARCH_IS_PPC64(def->os.arch)) {
+if (ARCH_IS_PPC64(def->os.arch))
 return VIR_DOMAIN_VIDEO_TYPE_VGA;
-} else if (qemuDomainIsARMVirt(def) ||
-   qemuDomainIsRISCVVirt(def) ||
-   ARCH_IS_S390(def->os.arch)) {
+if (qemuDomainIsARMVirt(def) || qemuDomainIsRISCVVirt(def) ||
+ARCH_IS_S390(def->os.arch))
 return VIR_DOMAIN_VIDEO_TYPE_VIRTIO;
-} else {
-if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA))
-return VIR_DOMAIN_VIDEO_TYPE_CIRRUS;
-if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA))
-return VIR_DOMAIN_VIDEO_TYPE_VGA;
-return VIR_DOMAIN_VIDEO_TYPE_DEFAULT;
-}
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA))
+return VIR_DOMAIN_VIDEO_TYPE_CIRRUS;
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA))
+return VIR_DOMAIN_VIDEO_TYPE_VGA;
+return VIR_DOMAIN_VIDEO_TYPE_DEFAULT;
 }
 
 
-- 
2.21.0

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

Re: [libvirt] [RFC PATCH 0/9] Introduce mediate ops in vfio-pci

[Jason Wang]

On 2019/12/6 下午4:22, Yan Zhao wrote:

On Thu, Dec 05, 2019 at 09:05:54PM +0800, Jason Wang wrote:

On 2019/12/5 下午4:51, Yan Zhao wrote:

On Thu, Dec 05, 2019 at 02:33:19PM +0800, Jason Wang wrote:

Hi:

On 2019/12/5 上午11:24, Yan Zhao wrote:

For SRIOV devices, VFs are passthroughed into guest directly without host
driver mediation. However, when VMs migrating with passthroughed VFs,
dynamic host mediation is required to  (1) get device states, (2) get
dirty pages. Since device states as well as other critical information
required for dirty page tracking for VFs are usually retrieved from PFs,
it is handy to provide an extension in PF driver to centralizingly control
VFs' migration.

Therefore, in order to realize (1) passthrough VFs at normal time, (2)
dynamically trap VFs' bars for dirty page tracking and

A silly question, what's the reason for doing this, is this a must for dirty
page tracking?


For performance consideration. VFs' bars should be passthoughed at
normal time and only enter into trap state on need.


Right, but how does this matter for the case of dirty page tracking?


Take NIC as an example, to trap its VF dirty pages, software way is
required to trap every write of ring tail that resides in BAR0.



Interesting, but it looks like we need:
- decode the instruction
- mediate all access to BAR0
All of which seems a great burden for the VF driver. I wonder whether or 
not doing interrupt relay and tracking head is better in this case.




  There's
still no IOMMU Dirty bit available.

(3) centralizing
VF critical states retrieving and VF controls into one driver, we propose
to introduce mediate ops on top of current vfio-pci device driver.


  _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
__   register mediate ops|  ___ ___|
|  |<---| VF|   |   |
| vfio-pci |  | |  mediate  |   | PF driver |   |
|__|--->|   driver  |   |___|
|open(pdev)  |  ---  | |
||
||_ _ _ _ _ _ _ _ _ _ _ _|_ _ _ _ _|
   \|/  \|/
--- 
|VF   | |PF|
--- 


VF mediate driver could be a standalone driver that does not bind to
any devices (as in demo code in patches 5-6) or it could be a built-in
extension of PF driver (as in patches 7-9) .

Rather than directly bind to VF, VF mediate driver register a mediate
ops into vfio-pci in driver init. vfio-pci maintains a list of such
mediate ops.
(Note that: VF mediate driver can register mediate ops into vfio-pci
before vfio-pci binding to any devices. And VF mediate driver can
support mediating multiple devices.)

When opening a device (e.g. a VF), vfio-pci goes through the mediate ops
list and calls each vfio_pci_mediate_ops->open() with pdev of the opening
device as a parameter.
VF mediate driver should return success or failure depending on it
supports the pdev or not.
E.g. VF mediate driver would compare its supported VF devfn with the
devfn of the passed-in pdev.
Once vfio-pci finds a successful vfio_pci_mediate_ops->open(), it will
stop querying other mediate ops and bind the opening device with this
mediate ops using the returned mediate handle.

Further vfio-pci ops (VFIO_DEVICE_GET_REGION_INFO ioctl, rw, mmap) on the
VF will be intercepted into VF mediate driver as
vfio_pci_mediate_ops->get_region_info(),
vfio_pci_mediate_ops->rw,
vfio_pci_mediate_ops->mmap, and get customized.
For vfio_pci_mediate_ops->rw and vfio_pci_mediate_ops->mmap, they will
further return 'pt' to indicate whether vfio-pci should further
passthrough data to hw.

when vfio-pci closes the VF, it calls its vfio_pci_mediate_ops->release()
with a mediate handle as parameter.

The mediate handle returned from vfio_pci_mediate_ops->open() lets VF
mediate driver be able to differentiate two opening VFs of the same device
id and vendor id.

When VF mediate driver exits, it unregisters its mediate ops from
vfio-pci.


In this patchset, we enable vfio-pci to provide 3 things:
(1) calling mediate ops to allow vendor driver customizing default
region info/rw/mmap of a region.
(2) provide a migration region to support migration

What's the benefit of introducing a region? It looks to me we don't expect
the region to be accessed directly from guest. Could we simply extend device
fd ioctl for doing such things?


You may take a look on mdev live migration discussions in
https://lists.gnu.org/archive/html/qemu-devel/2019-11/msg01763.html

or previous discussion at
https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg04908.html,
which has kernel side implemetation 
https://patchwork.free

[libvirt] [PATCH] qemu_monitor_text: Drop unused variable and avoid crash

[Michal Privoznik]

In v5.8.0-rc1~122 we've removed the only use of @safename in
qemuMonitorTextLoadSnapshot(). What we are left with is an
declared but not initialized variable that is passed to
VIR_FREE().

Caught by libvirt-php test suite.

Signed-off-by: Michal Privoznik 
---

Pushed under trivial rule.

 src/qemu/qemu_monitor_text.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 9054682d60..b387235821 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -161,7 +161,6 @@ int qemuMonitorTextLoadSnapshot(qemuMonitorPtr mon, const 
char *name)
 char *cmd = NULL;
 char *reply = NULL;
 int ret = -1;
-char *safename;
 
 cmd = g_strdup_printf("loadvm \"%s\"", name);
 
@@ -194,7 +193,6 @@ int qemuMonitorTextLoadSnapshot(qemuMonitorPtr mon, const 
char *name)
 ret = 0;
 
  cleanup:
-VIR_FREE(safename);
 VIR_FREE(cmd);
 VIR_FREE(reply);
 return ret;
-- 
2.23.0

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

Re: [libvirt] [PATCH v2 21/25] qemu: backup: Implement stats gathering while the job is running

[Peter Krempa]

On Thu, Dec 05, 2019 at 13:54:33 -0600, Eric Blake wrote:
> On 12/3/19 11:17 AM, Peter Krempa wrote:
> > We can use the output of 'query-jobs' to figure out some useful
> > information about a backup job. That is progress in case of a push job
> > and scratch file use in case of a pull job.
> 
> [I still need to finish my review on 20, but this one's easier to knock out
> in the short term]
> 
> > 
> > Add a worker which will total up the data and call it from
> > qemuDomainGetJobStatsInternal.
> > 
> > Signed-off-by: Peter Krempa 
> > ---
> 
> 
> > +static void
> > +qemuBackupGetJobInfoStatsUpdateOne(virDomainObjPtr vm,
> 
> > +
> > +if (push) {
> > +stats->total += monblockjob->progressTotal;
> > +stats->transferred += monblockjob->progressCurrent;
> > +} else {
> > +stats->tmp_used += monblockjob->progressCurrent;
> > +stats->tmp_total += monblockjob->progressTotal;
> > +}
> 
> I don't know what qemu reports for the job used/total on the temp image (I
> guess the total is the guest-visible disk size?) but this reporting is
> reasonable enough for now.

Yes it's the guest visible size, which is basically the maximum amount
that can be written into the scratch image in the current impl. In case
you add the flag you suggested earlier which will make the blocks not
covered by backup inaccessible and thus optimize the size of the scratch
image the job stats reported by qemu should reflect that.

> This patch looks fine, so:
> Reviewed-by: Eric Blake 
> 
> Still, seeing it made me wonder - when I first wrote the bitmap code, I
> added flag VIR_DOMAIN_CHECKPOINT_XML_SIZE for grabbing the current size of a
> bitmap, but disabled the qemu implementation of it at the last minute when
> committing the Checkpoint API because the monitor actions I used to get at
> it before -blockdev was not sane.  But I don't see it supported anywhere in
> this series.  The progress stats of a backup job are similar, but at some
> point, we do need to get a followup patch that gets size estimation from a
> bitmap prior to starting the backup back to viability.

I'm holding that patch until the integration with snapshots is done as
it requires merging of the bitmaps to figure out the size of the backup. 

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

[libvirt] [PATCH 0/3] qemu: fix crash bugs in snapshot revert

[Pavel Mores]

The aim of this series is to fix

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

however before getting to that we first need to fix an unrelated (and much
more recent) bug in patch 1.  We clean up the fix in patch 2 by converting the
whole function to the new allocation idioms.

The actual reported bug is then fixed in patch 3.

Pavel Mores (3):
  qemu: fix crash due to freeing an uninitialised pointer
  qemu: use g_autofree instead of VIR_FREE in
qemuMonitorTextCreateSnapshot()
  qemu: fix concurrency crash bug in snapshot revert

 src/qemu/qemu_driver.c   | 17 ++---
 src/qemu/qemu_monitor_text.c | 20 ++--
 2 files changed, 20 insertions(+), 17 deletions(-)

-- 
2.21.0

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

[libvirt] [PATCH 2/3] qemu: use g_autofree instead of VIR_FREE in qemuMonitorTextCreateSnapshot()

[Pavel Mores]

While at bugfixing, convert the whole function to the new-style memory
allocation handling.

Signed-off-by: Pavel Mores 
---
 src/qemu/qemu_monitor_text.c | 18 ++
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index b387235821..7586ba4c54 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -125,14 +125,13 @@ int
 qemuMonitorTextCreateSnapshot(qemuMonitorPtr mon,
   const char *name)
 {
-char *cmd = NULL;
-char *reply = NULL;
-int ret = -1;
+g_autofree char *cmd = NULL;
+g_autofree char *reply = NULL;
 
 cmd = g_strdup_printf("savevm \"%s\"", name);
 
 if (qemuMonitorJSONHumanCommand(mon, cmd, &reply))
-goto cleanup;
+return -1;
 
 if (strstr(reply, "Error while creating snapshot") ||
 strstr(reply, "Could not open VM state file") ||
@@ -141,19 +140,14 @@ qemuMonitorTextCreateSnapshot(qemuMonitorPtr mon,
 (strstr(reply, "Error") && strstr(reply, "while writing VM"))) {
 virReportError(VIR_ERR_OPERATION_FAILED,
_("Failed to take snapshot: %s"), reply);
-goto cleanup;
+return -1;
 } else if (strstr(reply, "No block device can accept snapshots")) {
 virReportError(VIR_ERR_OPERATION_INVALID, "%s",
_("this domain does not have a device to take 
snapshots"));
-goto cleanup;
+return -1;
 }
 
-ret = 0;
-
- cleanup:
-VIR_FREE(cmd);
-VIR_FREE(reply);
-return ret;
+return 0;
 }
 
 int qemuMonitorTextLoadSnapshot(qemuMonitorPtr mon, const char *name)
-- 
2.21.0

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

[libvirt] [PATCH 3/3] qemu: fix concurrency crash bug in snapshot revert

[Pavel Mores]

Although qemuDomainRevertToSnapshot() correctly begins a job at the start,
the job is implicitly ended if qemuProcessStop() is called because it lives
in the QEMU driver's private data that is purged during qemuProcessStop().
This commit restores the job by calling qemuProcessBeginJob() after
qemuProcessStop() invocations.

The first chunk is meant to fix a reported bug (see below).  The bug's
reproducibility on my machine was initially way lower than the reported
50% (more like 5%) but could be boosted to pretty much 100% by having
virt-manager connected, displaying the testing domain in viewer.  With the
fix, the bug cannot be reproduced on my machine under any scenario I could
think of.

The actual crash was due to the thread performing revert which, not
acquiring a job properly, was able to change the QEMU monitor structure
under the thread performing snapshot delete while it was waiting on a
condition variable.

An interesting question is whether this commit takes the right approach
to the fix.  In particular, qemuProcessStop() arguably should not clear
jobs, in which case the right thing to do would be to fix
qemuProcessStop().  However, qemuProcessStop() has about twenty callers,
and while none of them seems vulnerable to the kind of problems caused by
clearing jobs (unlike qemuDomainRevertToSnapshot() who starts QEMU again
right after stopping it), some of them might rely on jobs being cleared
(this is not always easy to check conclusively).  All in all, this commit's
solution seems to be the least bad fix which doesn't require a potentially
risky refactor.

https://bugzilla.redhat.com/show_bug.cgi?id=1610207
Signed-off-by: Pavel Mores 
---
 src/qemu/qemu_driver.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 18bd0101e7..b3769cc479 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16637,9 +16637,15 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
snapshot,
  VIR_DOMAIN_EVENT_STOPPED,
  detail);
 virObjectEventStateQueue(driver->domainEventState, event);
-/* Start after stop won't be an async start job, so
- * reset to none */
-jobType = QEMU_ASYNC_JOB_NONE;
+/* We have to begin a new job as the original one (begun
+ * near the top of this function) was lost during the purge
+ * of private data in qemuProcessStop() above.
+ */
+if (qemuProcessBeginJob(driver, vm,
+
VIR_DOMAIN_JOB_OPERATION_SNAPSHOT_REVERT,
+flags) < 0) {
+goto cleanup;
+}
 goto load;
 }
 }
@@ -16774,6 +16780,11 @@ qemuDomainRevertToSnapshot(virDomainSnapshotPtr 
snapshot,
 event = virDomainEventLifecycleNewFromObj(vm,
  VIR_DOMAIN_EVENT_STOPPED,
  detail);
+if (qemuProcessBeginJob(driver, vm,
+VIR_DOMAIN_JOB_OPERATION_SNAPSHOT_REVERT,
+flags) < 0) {
+goto cleanup;
+}
 }
 
 if (qemuDomainSnapshotRevertInactive(driver, vm, snap) < 0) {
-- 
2.21.0

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

[libvirt] [PATCH 1/3] qemu: fix crash due to freeing an uninitialised pointer

[Pavel Mores]

The bug was fairly though not 100% reproducible, presumably due to the fact
that some of the time, 'safename' might turn out NULL by chance, in which case
freeing it is OK.

Signed-off-by: Pavel Mores 
---
 src/qemu/qemu_monitor_text.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 9054682d60..b387235821 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -161,7 +161,6 @@ int qemuMonitorTextLoadSnapshot(qemuMonitorPtr mon, const 
char *name)
 char *cmd = NULL;
 char *reply = NULL;
 int ret = -1;
-char *safename;
 
 cmd = g_strdup_printf("loadvm \"%s\"", name);
 
@@ -194,7 +193,6 @@ int qemuMonitorTextLoadSnapshot(qemuMonitorPtr mon, const 
char *name)
 ret = 0;
 
  cleanup:
-VIR_FREE(safename);
 VIR_FREE(cmd);
 VIR_FREE(reply);
 return ret;
-- 
2.21.0

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

Re: [libvirt] [PATCH] news: Update the qemu version that dedicated performance hint is from

[Andrea Bolognani]

On Fri, 2019-12-06 at 10:50 +0800, Han Han wrote:
> KVM dedicated performance hint is added since qemu version 2.10.0 not
> 2.10.1.
> 
> Signed-off-by: Han Han 
> ---
>  docs/news.xml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/docs/news.xml b/docs/news.xml
> index 2eb3bfdf6e..3ddebfddfb 100644
> --- a/docs/news.xml
> +++ b/docs/news.xml
> @@ -469,7 +469,7 @@
>With  and
>, it
>allows a guest to enable optimizations when running on dedicated
> -  vCPUs. QEMU newer than 2.12.1 and kernel newer than 4.17
> +  vCPUs. QEMU newer than 2.12.0 and kernel newer than 4.17
>are required.
>  
>

Reviewed-by: Andrea Bolognani 

and pushed. Thanks for pointing out the mistake in the original
commit! I've updated docs/formatdomain.html as well in a separate
patch.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[libvirt] [PATCH] docs: Update minimum QEMU version for kvm-hint-dedicated

[Andrea Bolognani]

Same fix that was applied to release notes in a595c66a134d.

Signed-off-by: Andrea Bolognani 
---
Pushed as trivial.

 docs/formatdomain.html.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 6df4a8b26e..5c0a00350b 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2226,7 +2226,7 @@
   hint-dedicated
   Allows a guest to enable optimizations when running on dedicated 
vCPUs
   on, off
-  5.7.0 (QEMU 2.12.1)
+  5.7.0 (QEMU 2.12.0)
 
   
   
-- 
2.23.0

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

Re: [libvirt] [RFC PATCH 0/9] Introduce mediate ops in vfio-pci

[Yan Zhao]

On Thu, Dec 05, 2019 at 09:05:54PM +0800, Jason Wang wrote:
> 
> On 2019/12/5 下午4:51, Yan Zhao wrote:
> > On Thu, Dec 05, 2019 at 02:33:19PM +0800, Jason Wang wrote:
> >> Hi:
> >>
> >> On 2019/12/5 上午11:24, Yan Zhao wrote:
> >>> For SRIOV devices, VFs are passthroughed into guest directly without host
> >>> driver mediation. However, when VMs migrating with passthroughed VFs,
> >>> dynamic host mediation is required to  (1) get device states, (2) get
> >>> dirty pages. Since device states as well as other critical information
> >>> required for dirty page tracking for VFs are usually retrieved from PFs,
> >>> it is handy to provide an extension in PF driver to centralizingly control
> >>> VFs' migration.
> >>>
> >>> Therefore, in order to realize (1) passthrough VFs at normal time, (2)
> >>> dynamically trap VFs' bars for dirty page tracking and
> >>
> >> A silly question, what's the reason for doing this, is this a must for 
> >> dirty
> >> page tracking?
> >>
> > For performance consideration. VFs' bars should be passthoughed at
> > normal time and only enter into trap state on need.
> 
> 
> Right, but how does this matter for the case of dirty page tracking?
>
Take NIC as an example, to trap its VF dirty pages, software way is
required to trap every write of ring tail that resides in BAR0. There's
still no IOMMU Dirty bit available.
> 
> >
> >>>(3) centralizing
> >>> VF critical states retrieving and VF controls into one driver, we propose
> >>> to introduce mediate ops on top of current vfio-pci device driver.
> >>>
> >>>
> >>>  _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
> >>>__   register mediate ops|  ___ ___|
> >>> |  |<---| VF|   |   |
> >>> | vfio-pci |  | |  mediate  |   | PF driver |   |
> >>> |__|--->|   driver  |   |___|
> >>>|open(pdev)  |  ---  | |
> >>>||
> >>>||_ _ _ _ _ _ _ _ _ _ _ _|_ _ _ _ _|
> >>>   \|/  \|/
> >>> --- 
> >>> |VF   | |PF|
> >>> --- 
> >>>
> >>>
> >>> VF mediate driver could be a standalone driver that does not bind to
> >>> any devices (as in demo code in patches 5-6) or it could be a built-in
> >>> extension of PF driver (as in patches 7-9) .
> >>>
> >>> Rather than directly bind to VF, VF mediate driver register a mediate
> >>> ops into vfio-pci in driver init. vfio-pci maintains a list of such
> >>> mediate ops.
> >>> (Note that: VF mediate driver can register mediate ops into vfio-pci
> >>> before vfio-pci binding to any devices. And VF mediate driver can
> >>> support mediating multiple devices.)
> >>>
> >>> When opening a device (e.g. a VF), vfio-pci goes through the mediate ops
> >>> list and calls each vfio_pci_mediate_ops->open() with pdev of the opening
> >>> device as a parameter.
> >>> VF mediate driver should return success or failure depending on it
> >>> supports the pdev or not.
> >>> E.g. VF mediate driver would compare its supported VF devfn with the
> >>> devfn of the passed-in pdev.
> >>> Once vfio-pci finds a successful vfio_pci_mediate_ops->open(), it will
> >>> stop querying other mediate ops and bind the opening device with this
> >>> mediate ops using the returned mediate handle.
> >>>
> >>> Further vfio-pci ops (VFIO_DEVICE_GET_REGION_INFO ioctl, rw, mmap) on the
> >>> VF will be intercepted into VF mediate driver as
> >>> vfio_pci_mediate_ops->get_region_info(),
> >>> vfio_pci_mediate_ops->rw,
> >>> vfio_pci_mediate_ops->mmap, and get customized.
> >>> For vfio_pci_mediate_ops->rw and vfio_pci_mediate_ops->mmap, they will
> >>> further return 'pt' to indicate whether vfio-pci should further
> >>> passthrough data to hw.
> >>>
> >>> when vfio-pci closes the VF, it calls its vfio_pci_mediate_ops->release()
> >>> with a mediate handle as parameter.
> >>>
> >>> The mediate handle returned from vfio_pci_mediate_ops->open() lets VF
> >>> mediate driver be able to differentiate two opening VFs of the same device
> >>> id and vendor id.
> >>>
> >>> When VF mediate driver exits, it unregisters its mediate ops from
> >>> vfio-pci.
> >>>
> >>>
> >>> In this patchset, we enable vfio-pci to provide 3 things:
> >>> (1) calling mediate ops to allow vendor driver customizing default
> >>> region info/rw/mmap of a region.
> >>> (2) provide a migration region to support migration
> >>
> >> What's the benefit of introducing a region? It looks to me we don't expect
> >> the region to be accessed directly from guest. Could we simply extend 
> >> device
> >> fd ioctl for doing such things?
> >>
> > You may take a look on mdev live migration

Re: [libvirt] [PATCH 0/3] Remove deprecated pc-0.x machine types and related hacks

[Thomas Huth]

On 06/12/2019 07.49, Markus Armbruster wrote:
> Thomas Huth  writes:
> 
>> These have been on the deprecation list since a year now, so it's
>> time to finally remove the pc-0.x machine types.
>>
>> We then can also remove some compatibility hacks in the devices, i.e.
>> the "use_broken_id" in ac97 and "command_serr_enable" in PCI devices.
>>
>> Note that there is also the "rombar" property of the PCI devices which
>> is now not required for the x86 machine types anymore. But it seems to
>> me like this is still used by various people to bypass the ROM loading
>> for PCI devices in certain cases, so I did not remove that property here
>> yet.
> 
> With this series applied:
> 
> $ git-grep pc-0
> hw/display/vga-pci.c:/* compatibility with pc-0.13 and older */
> hw/display/vga.c:/* With pc-0.12 and below we map both the PCI BAR and 
> the fixed VBE region,
> hw/display/vmware_vga.c:/* compatibility with pc-0.13 and older */

These are the "rombar" hacks that I've mentioned above. The question is
whether we want to remove them or whether I should just adjust the comments?

> hw/i386/pc_piix.c:/* PC compat function

Right, the comment still needs to be adjusted.

 Thomas

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

Re: [libvirt] [RFC PATCH 1/9] vfio/pci: introduce mediate ops to intercept vfio-pci ops

[Yan Zhao]

On Fri, Dec 06, 2019 at 07:55:19AM +0800, Alex Williamson wrote:
> On Wed,  4 Dec 2019 22:25:36 -0500
> Yan Zhao  wrote:
> 
> > when vfio-pci is bound to a physical device, almost all the hardware
> > resources are passthroughed.
> > Sometimes, vendor driver of this physcial device may want to mediate some
> > hardware resource access for a short period of time, e.g. dirty page
> > tracking during live migration.
> > 
> > Here we introduce mediate ops in vfio-pci for this purpose.
> > 
> > Vendor driver can register a mediate ops to vfio-pci.
> > But rather than directly bind to the passthroughed device, the
> > vendor driver is now either a module that does not bind to any device or
> > a module binds to other device.
> > E.g. when passing through a VF device that is bound to vfio-pci modules,
> > PF driver that binds to PF device can register to vfio-pci to mediate
> > VF's regions, hence supporting VF live migration.
> > 
> > The sequence goes like this:
> > 1. Vendor driver register its vfio_pci_mediate_ops to vfio-pci driver
> > 
> > 2. vfio-pci maintains a list of those registered vfio_pci_mediate_ops
> > 
> > 3. Whenever vfio-pci opens a device, it searches the list and call
> > vfio_pci_mediate_ops->open() to check whether a vendor driver supports
> > mediating this device.
> > Upon a success return value of from vfio_pci_mediate_ops->open(),
> > vfio-pci will stop list searching and store a mediate handle to
> > represent this open into vendor driver.
> > (so if multiple vendor drivers support mediating a device through
> > vfio_pci_mediate_ops, only one will win, depending on their registering
> > sequence)
> > 
> > 4. Whenever a VFIO_DEVICE_GET_REGION_INFO ioctl is received in vfio-pci
> > ops, it will chain into vfio_pci_mediate_ops->get_region_info(), so that
> > vendor driver is able to override a region's default flags and caps,
> > e.g. adding a sparse mmap cap to passthrough only sub-regions of a whole
> > region.
> > 
> > 5. vfio_pci_rw()/vfio_pci_mmap() first calls into
> > vfio_pci_mediate_ops->rw()/vfio_pci_mediate_ops->mmaps().
> > if pt=true is rteturned, vfio_pci_rw()/vfio_pci_mmap() will further
> > passthrough this read/write/mmap to physical device, otherwise it just
> > returns without touch physical device.
> > 
> > 6. When vfio-pci closes a device, vfio_pci_release() chains into
> > vfio_pci_mediate_ops->release() to close the reference in vendor driver.
> > 
> > 7. Vendor driver unregister its vfio_pci_mediate_ops when driver exits
> > 
> > Cc: Kevin Tian 
> > 
> > Signed-off-by: Yan Zhao 
> > ---
> >  drivers/vfio/pci/vfio_pci.c | 146 
> >  drivers/vfio/pci/vfio_pci_private.h |   2 +
> >  include/linux/vfio.h|  16 +++
> >  3 files changed, 164 insertions(+)
> > 
> > diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> > index 02206162eaa9..55080ff29495 100644
> > --- a/drivers/vfio/pci/vfio_pci.c
> > +++ b/drivers/vfio/pci/vfio_pci.c
> > @@ -54,6 +54,14 @@ module_param(disable_idle_d3, bool, S_IRUGO | S_IWUSR);
> >  MODULE_PARM_DESC(disable_idle_d3,
> >  "Disable using the PCI D3 low power state for idle, unused 
> > devices");
> >  
> > +static LIST_HEAD(mediate_ops_list);
> > +static DEFINE_MUTEX(mediate_ops_list_lock);
> > +struct vfio_pci_mediate_ops_list_entry {
> > +   struct vfio_pci_mediate_ops *ops;
> > +   int refcnt;
> > +   struct list_headnext;
> > +};
> > +
> >  static inline bool vfio_vga_disabled(void)
> >  {
> >  #ifdef CONFIG_VFIO_PCI_VGA
> > @@ -472,6 +480,10 @@ static void vfio_pci_release(void *device_data)
> > if (!(--vdev->refcnt)) {
> > vfio_spapr_pci_eeh_release(vdev->pdev);
> > vfio_pci_disable(vdev);
> > +   if (vdev->mediate_ops && vdev->mediate_ops->release) {
> > +   vdev->mediate_ops->release(vdev->mediate_handle);
> > +   vdev->mediate_ops = NULL;
> > +   }
> > }
> >  
> > mutex_unlock(&vdev->reflck->lock);
> > @@ -483,6 +495,7 @@ static int vfio_pci_open(void *device_data)
> >  {
> > struct vfio_pci_device *vdev = device_data;
> > int ret = 0;
> > +   struct vfio_pci_mediate_ops_list_entry *mentry;
> >  
> > if (!try_module_get(THIS_MODULE))
> > return -ENODEV;
> > @@ -495,6 +508,30 @@ static int vfio_pci_open(void *device_data)
> > goto error;
> >  
> > vfio_spapr_pci_eeh_open(vdev->pdev);
> > +   mutex_lock(&mediate_ops_list_lock);
> > +   list_for_each_entry(mentry, &mediate_ops_list, next) {
> > +   u64 caps;
> > +   u32 handle;
> 
> Wouldn't it seem likely that the ops provider might use this handle as
> a pointer, so we'd want it to be an opaque void*?
>
yes, you are right, handle as a pointer is much better. will change it.
Thanks :)

> > +
> > +   memset(&caps, 0, sizeof(caps));
> 
> @caps has no purpose