Re: [libvirt] [PATCH] qemu: hotplug: fix mdev attach for vfio-ccw

[Bjoern Walk]

John Ferlan  [2018-06-26, 03:03PM -0400]:
> 
> 
> On 06/26/2018 07:47 AM, Bjoern Walk wrote:
> > Mediated devices of model 'vfio-ccw' are using CCW addresses, so make
> > sure to call the correct address preparation code for the model.
> > 
> > Reviewed-by: Shalini Chellathurai Saroja 
> > Reviewed-by: Boris Fiuczynski 
> > Signed-off-by: Bjoern Walk 
> > ---
> >  src/qemu/qemu_hotplug.c | 11 +--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> 
> Reviewed-by: John Ferlan 
> (and pushed)

Thanks a bunch!

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

-- 
IBM Systems
Linux on Z & Virtualization Development

IBM Deutschland Research & Development GmbH
Schönaicher Str. 220, 71032 Böblingen
Phone: +49 7031 16 1819

Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294 


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

Re: [libvirt] [PATCH 4/8] backup: Document new XML for backups

[Eric Blake]

On 06/26/2018 02:51 PM, Nir Soffer wrote:

On Wed, Jun 13, 2018 at 7:42 PM Eric Blake  wrote:


Prepare for new checkpoint and backup APIs by describing the XML
that will represent a checkpoint.  This is modeled heavily after
the XML for virDomainSnapshotPtr, since both represent a point in
time of the guest.  But while a snapshot exists with the intent
of rolling back to that state, a checkpoint instead makes it
possible to create an incremental backup at a later time.

Add testsuite coverage of a minimal use of the XML.



+++ b/docs/formatcheckpoint.html.in
@@ -0,0 +1,273 @@
+
+
+http://www.w3.org/1999/xhtml;>
+  
+Checkpoint and Backup XML format
+
+
+
+Checkpoint XML



id=CheckpointXML?


Matches what the existing formatsnapshot.html.in named its tag. (If you 
haven't guessed, I'm heavily relying on snapshots as my template for 
adding this).






+
+
+  Domain disk backups, including incremental backups, are one form
+  of domain state capture.
+
+
+  Libvirt is able to facilitate incremental backups by tracking
+  disk checkpoints, or points in time against which it is easy to
+  compute which portion of the disk has changed.  Given a full
+  backup (a backup created from the creation of the disk to a
+  given point in time, coupled with the creation of a disk
+  checkpoint at that time),



Not clear.



and an incremental backup (a backup
+  created from just the dirty portion of the disk between the
+  first checkpoint and the second backup operation),



Also not clear.


Okay, I will try to improve these in v2.  But (other than answering 
these good review emails), my current priority is a working demo (to 
prove the API works) prior to further doc polish.






it is
+  possible to do an offline reconstruction of the state of the
+  disk at the time of the second backup, without having to copy as
+  much data as a second full backup would require.  Most disk
+  checkpoints are created in concert with a backup,
+  via virDomainBackupBegin(); however, libvirt also
+  exposes enough support to create disk checkpoints independently
+  from a backup operation,
+  via virDomainCheckpointCreateXML().



Thanks for the extra context.



+
+
+  Attributes of libvirt checkpoints are stored as child elements of
+  the domaincheckpoint element.  At checkpoint creation
+  time, normally only the name, description,
+  and disks elements are settable; the rest of the
+  fields are ignored on creation, and will be filled in by
+  libvirt in for informational purposes



So the user is responsible for creating checkpoints names? Do we use these
the same name in qcow2?


My intent is that if the user does not assign a checkpoint name, then 
libvirt will default it to the current time in seconds-since-the-Epoch. 
Then, whatever name is given to the checkpoint (whether chosen by 
libvirt or assigned by the user) will also be the default name of the 
bitmap created in each qcow2 volume, but the XML also allows you to name 
the qcow2 bitmaps something different than the checkpoint name (maybe 
not a wise idea in the common case, but could come in handy later if you 
use the _REDEFINE flag to teach libvirt about existing bitmaps that are 
already present in a qcow2 image rather than placed there by libvirt).



+
+  Checkpoints are maintained in a hierarchy.  A domain can have a
+  current checkpoint, which is the most recent checkpoint compared to
+  the current state of the domain (although a domain might have
+  checkpoints without a current checkpoint, if checkpoints have been
+  deleted in the meantime).  Creating or reverting to a checkpoint
+  sets that checkpoint as current, and the prior current checkpoint is
+  the parent of the new checkpoint.  Branches in the hierarchy can
+  be formed by reverting to a checkpoint with a child, then creating
+  another checkpoint.



This seems too complex. Why do we need arbitrary trees of checkpoints?


Because snapshots had an arbitrary tree, and it was easier to copy from 
snapshots. Even if we only use a linear tree for now, it is still 
feasible that in the future, we can facilitate a domain rolling back to 
the disk state as captured at checkpoint C1, at which point you could 
then have multiple children C2 (the bitmap created prior to rolling 
back) and C3 (the bitmap created for tracking changes made after rolling 
back).  Again, for a first cut, I probably will punt and state that 
snapshots and incremental backups do not play well together yet; but as 
we get experience and add more code, the API is flexible enough that 
down the road we really can offer reverting to an arbitrary snapshot and 
ALSO updating checkpoints to match.




What is the meaning of reverting a checkpoint?


Hmm - right now, you can't (that was one Snapshot API that I 
intentionally did not copy over to Checkpoint), so 

Re: [libvirt] [PATCH 3/8] backup: Introduce virDomainCheckpointPtr

[Eric Blake]

On 06/26/2018 02:03 PM, Nir Soffer wrote:

On Wed, Jun 13, 2018 at 7:42 PM Eric Blake  wrote:


Prepare for introducing a bunch of new public APIs related to
backup checkpoints by first introducing a new internal type
and errors associated with that type.  Checkpoints are modeled
heavily after virDomainSnapshotPtr (both represent a point in
time of the guest), although a snapshot exists with the intent
of rolling back to that state, while a checkpoint exists to
make it possible to create an incremental backup at a later
time.

Signed-off-by: Eric Blake 
---
  include/libvirt/libvirt-domain-snapshot.h |  8 ++--
  include/libvirt/libvirt.h |  2 +
  include/libvirt/virterror.h   |  5 ++-
  src/datatypes.c   | 62
++-
  src/datatypes.h   | 31 +++-
  src/libvirt_private.syms  |  2 +
  src/util/virerror.c   | 15 +++-
  7 files changed, 118 insertions(+), 7 deletions(-)

diff --git a/include/libvirt/libvirt-domain-snapshot.h
b/include/libvirt/libvirt-domain-snapshot.h
index e5a893a767..ff1e890cfc 100644
--- a/include/libvirt/libvirt-domain-snapshot.h
+++ b/include/libvirt/libvirt-domain-snapshot.h
@@ -31,15 +31,17 @@
  /**
   * virDomainSnapshot:
   *
- * a virDomainSnapshot is a private structure representing a snapshot of
- * a domain.
+ * A virDomainSnapshot is a private structure representing a snapshot of
+ * a domain.  A snapshot captures the state of the domain at a point in
+ * time, with the intent that the guest can be reverted back to that
+ * state at a later time.



The extra context is very nice...



   */
  typedef struct _virDomainSnapshot virDomainSnapshot;

  /**
   * virDomainSnapshotPtr:
   *
- * a virDomainSnapshotPtr is pointer to a virDomainSnapshot private
structure,
+ * A virDomainSnapshotPtr is pointer to a virDomainSnapshot private
structure,
   * and is the type used to reference a domain snapshot in the API.
   */



But I think users of this API would like to find it here, explaining the
public
type.


That's a pre-existing documentation issue (probably worth a separate 
cleanup patch to a lot of files, if it really does render better to tie 
the details to the 'Ptr' typedef rather than the opaque typedef).



@@ -321,6 +322,8 @@ typedef enum {
 to guest-sync command
(DEPRECATED)*/
  VIR_ERR_LIBSSH = 98,/* error in libssh transport
driver */
  VIR_ERR_DEVICE_MISSING = 99,/* fail to find the desired
device */
+VIR_ERR_INVALID_DOMAIN_CHECKPOINT = 100,/* invalid domain checkpoint
*/



What is invalid checkpoint? It would be nice if there would not be
such thing.


Copied from the existing VIR_ERR_INVALID_DOMAIN_SNAPSHOT. Sadly, there 
MUST be such a thing - it exists to (try and) catch bugs such as:


void *ptr = virAPI1() (which returns virDomainPtr)
virDomainCheckpointAPI2(ptr, ...) (which expects virDomainCheckpointPtr)

where you are passing in the wrong type, or such as:

virConnectPtr conn = virAPI1()
virDomainCheckpointPtr chk = virAPI2(conn)
virConnectClose(conn)
virDomainCheckpointAPI3(chk)

where you are passing in the right type but wrong order because the 
checkpoint depends on a connection that you have closed




Also the comment does not add anything.


Such is the life of copy-and-paste.  My excuse is that the code I copied 
from has the same sort of poor comment.



@@ -292,6 +293,21 @@ extern virClassPtr virAdmClientClass;
  } \
  } while (0)

+# define virCheckDomainCheckpointReturn(obj, retval) \
+do { \
+virDomainCheckpointPtr _check = (obj); \
+if (!virObjectIsClass(_check, virDomainCheckpointClass) || \
+!virObjectIsClass(_check->domain, virDomainClass) || \
+!virObjectIsClass(_check->domain->conn, virConnectClass)) { \
+virReportErrorHelper(VIR_FROM_DOMAIN_CHECKPOINT, \
+ VIR_ERR_INVALID_DOMAIN_CHECKPOINT, \
+ __FILE__, __FUNCTION__, __LINE__, \
+ __FUNCTION__); \



I guess that this is invalid domain checkpoint. Isn't this a generic
error, providing a pointer of the wrong type?


Yes, except that libvirt already has the practice of distinguishing 
error messages according to which type was expected.



@@ -712,6 +739,8 @@ virStreamPtr virGetStream(virConnectPtr conn);
  virNWFilterPtr virGetNWFilter(virConnectPtr conn,
const char *name,
const unsigned char *uuid);
+virDomainCheckpointPtr virGetDomainCheckpoint(virDomainPtr domain,
+  const char *name);



I guess this implemented and documented elsewhere.


This is a function for internal use only; it is not exported as a public 
function, but exists to mirror...







  virDomainSnapshotPtr 

[libvirt] [PATCH] nwfilter: variable 'obj' must be initialized inside nwfilterBindingCreateXML().

[Julio Faracco]

The function nwfilterBindingCreateXML() is failing to compile due to a
conditional branch which leads to a undefined 'obj' variable. So 'obj'
must have a initial value to avoid compilation errors. See the problem:

  CC   nwfilter/libvirt_driver_nwfilter_impl_la-nwfilter_driver.lo
nwfilter/nwfilter_driver.c:752:9: error: variable 'obj' is used uninitialized 
whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
if (virNWFilterBindingCreateXMLEnsureACL(conn, def) < 0)
^~~
nwfilter/nwfilter_driver.c:779:10: note: uninitialized use occurs here
if (!obj)
 ^~~
nwfilter/nwfilter_driver.c:752:5: note: remove the 'if' if its condition is 
always false
if (virNWFilterBindingCreateXMLEnsureACL(conn, def) < 0)
^~~~
nwfilter/nwfilter_driver.c:742:33: note: initialize the variable 'obj' to 
silence this warning
virNWFilterBindingObjPtr obj;
^
 = NULL

This commit initialized 'obj' with NULL to fix the error properly.

Signed-off-by: Julio Faracco 
---
 src/nwfilter/nwfilter_driver.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index d385b46f5f..ed34586105 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -739,8 +739,8 @@ nwfilterBindingCreateXML(virConnectPtr conn,
  const char *xml,
  unsigned int flags)
 {
-virNWFilterBindingObjPtr obj;
 virNWFilterBindingDefPtr def;
+virNWFilterBindingObjPtr obj = NULL;
 virNWFilterBindingPtr ret = NULL;
 
 virCheckFlags(0, NULL);
-- 
2.17.1

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

Re: [libvirt] [PATCH 2/8] backup: Document nuances between different state capture APIs

[Eric Blake]

On 06/26/2018 11:36 AM, Nir Soffer wrote:

On Wed, Jun 13, 2018 at 7:42 PM Eric Blake  wrote:


Upcoming patches will add support for incremental backups via
a new API; but first, we need a landing page that gives an
overview of capturing various pieces of guest state, and which
APIs are best suited to which tasks.








Needs blank line between list items for easier reading of the source.


Sure.



I think we should describe checkpoints before backups, since the
expected flow is:

- user start backup
- system create checkpoint using virDomainCheckpointCreateXML
- system query amount of data pointed by the previous checkpoint
   bitmaps
- system create temporary storage for the backup
- system starts backup using virDomainBackupBegin


I actually think it will be more common to create checkpoints via 
virDomainBackupBegin(), and not virDomainCheckpointCreateXML (the latter 
exists because it is easy, and may have a use independent from 
incremental backups, but it is the former that makes chains of 
incremental backups reliable).


That is, your first backup will be a full backup (no checkpoint as its 
start) but will create a checkpoint at the same time; then your second 
backup is an incremental backup (use the checkpoint created at the first 
backup as the start) and also creates a checkpoint in anticipation of a 
third incremental backup.


You do have an interesting step in there - the ability to query how much 
data is pointed to in the delta between two checkpoints (that is, before 
I actually create a backup, can I pre-guess how much data it will end up 
copying).  On the other hand, the size of the temporary storage for the 
backup is not related to the amount of data tracked in the bitmap. 
Expanding on the examples in my 1/8 reply to you:


At T3, we have:

S1: || <- S2: |---BBB--|
B1: ||B2: |---XXX--|
guest sees: |AAABBB--|

where by T4 we will have:

S1: || <- S2: |D--BBDD-|
B1: ||B2: |---XXX--|
  B3: |XXX-|
guest sees: |DAABBDD-|

Back at T3, using B2 as our dirty bitmap, there are two backup models we 
can pursue to get at the data tracked by that bitmap.


The first is push-model backup (blockdev-backup with "sync":"top" to the 
actual backup file) - qemu directly writes the |---BBB--| sequence into 
the destination file (based on the contents of B2), whether or not S2 is 
modified in the meantime; in this mode, qemu is smart enough to not 
bother copying clusters to the destination that were not in the bitmap. 
So the fact that B2 mentions 3 dirty clusters indeed proves to be the 
right size for the destination file.


The second is pull-model backup (blockdev-backup with "sync":"none" to a 
temporary file, coupled with a read-only NBD server on the temporary 
file that also exposes bitmap B2 via NBD_CMD_BLOCK_STATUS) - here, if 
qemu can guarantee that the client would read only dirty clusters, then 
it only has to write to the temporary file if the guest changes a 
cluster that was tracked in B2 (so at most the temporary file would 
contain |-B--| if the NBD client finishes before T4); but more 
likely, qemu will play conservative and write to the temporary file for 
ANY changes whether or not they are to areas covered by B2 (in which 
case the temporary file could contain |AB0-| for the three writes 
done by T4).  Or put another way, if qemu can guarantee a nice client, 
then the size of B2 probably overestimates the size of the temporary 
file; but if qemu plays conservative by assuming the client will read 
even portions of the file that weren't dirty, then keeping those reads 
constant will require the temporary file to be as large as the guest is 
able to dirty data while the backup continues, which may be far larger 
than the size of B2.  [And maybe this argues that we want a way for an 
NBD export to force EIO read errors for anything outside of the exported 
dirty bitmap, thus making the client play nice, so that the temporary 
file does not have to grow beyond the size of the bitmap - but that's a 
future feature request]



+Examples
+The following two sequences both capture the disk state of a
+  running guest, then complete with the guest running on its
+  original disk image; but with a difference that an unexpected
+  interruption during the first mode leaves a temporary wrapper
+  file that must be accounted for, while interruption of the
+  second mode has no impact to the guest.



This is not clear, I read this several times and I'm not sure what do
you mean here.


I'm trying to convey the point that with example 1...



Blank line between paragraphs



+1. Backup via temporary snapshot
+  
+virDomainFSFreeze()
+virDomainSnapshotCreateXML(VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY)


...if you are interrupted here, your  XML has changed to point 
to the snapshot file...



+virDomainFSThaw()
+third-party copy the backing file to backup storage # most time spent here

[libvirt] [PATCH 1/2] lxc: moving 'type' argument to avoid issues with mount() syscall.

[Julio Faracco]

This commit fixes a lots of mount calls inside lxc_container.c file. The
NULL value into 'type' argument is causing a memory issue. See commit
794b576c2b for more details. The best approach to fix it is moving NULL
to "none" filesytem.

Signed-off-by: Julio Faracco 
---
 src/lxc/lxc_container.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 665b93a0ac..3a1b2d6819 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -720,7 +720,7 @@ static int lxcContainerPivotRoot(virDomainFSDefPtr root)
 VIR_DEBUG("Pivot via %s", root->src->path);
 
 /* root->parent must be private, so make / private. */
-if (mount("", "/", NULL, MS_PRIVATE|MS_REC, NULL) < 0) {
+if (mount("", "/", "none", MS_PRIVATE|MS_REC, NULL) < 0) {
 virReportSystemError(errno, "%s",
  _("Failed to make root private"));
 goto err;
@@ -757,7 +757,7 @@ static int lxcContainerPivotRoot(virDomainFSDefPtr root)
 }
 
 /* ... and mount our root onto it */
-if (mount(root->src->path, newroot, NULL, MS_BIND|MS_REC, NULL) < 0) {
+if (mount(root->src->path, newroot, "none", MS_BIND|MS_REC, NULL) < 0) {
 virReportSystemError(errno,
  _("Failed to bind %s to new root %s"),
  root->src->path, newroot);
@@ -765,7 +765,7 @@ static int lxcContainerPivotRoot(virDomainFSDefPtr root)
 }
 
 if (root->readonly) {
-if (mount(root->src->path, newroot, NULL, 
MS_BIND|MS_REC|MS_RDONLY|MS_REMOUNT, NULL) < 0) {
+if (mount(root->src->path, newroot, "none", 
MS_BIND|MS_REC|MS_RDONLY|MS_REMOUNT, NULL) < 0) {
 virReportSystemError(errno,
  _("Failed to make new root %s readonly"),
  root->src->path);
@@ -815,9 +815,9 @@ typedef struct {
 
 static const virLXCBasicMountInfo lxcBasicMounts[] = {
 { "proc", "/proc", "proc", MS_NOSUID|MS_NOEXEC|MS_NODEV, false, false, 
false },
-{ "/proc/sys", "/proc/sys", NULL, 
MS_BIND|MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, false, false, false },
-{ "/.oldroot/proc/sys/net/ipv4", "/proc/sys/net/ipv4", NULL, MS_BIND, 
false, false, true },
-{ "/.oldroot/proc/sys/net/ipv6", "/proc/sys/net/ipv6", NULL, MS_BIND, 
false, false, true },
+{ "/proc/sys", "/proc/sys", "none", 
MS_BIND|MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, false, false, false },
+{ "/.oldroot/proc/sys/net/ipv4", "/proc/sys/net/ipv4", "none", MS_BIND, 
false, false, true },
+{ "/.oldroot/proc/sys/net/ipv6", "/proc/sys/net/ipv6", "none", MS_BIND, 
false, false, true },
 { "sysfs", "/sys", "sysfs", MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, false, 
false, false },
 { "securityfs", "/sys/kernel/security", "securityfs", 
MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RDONLY, true, true, false },
 #if WITH_SELINUX
@@ -876,7 +876,7 @@ static int lxcContainerSetReadOnly(void)
 
 for (i = 0; i < nmounts; i++) {
 VIR_DEBUG("Bind readonly %s", mounts[i]);
-if (mount(mounts[i], mounts[i], NULL, 
MS_BIND|MS_REC|MS_RDONLY|MS_REMOUNT, NULL) < 0) {
+if (mount(mounts[i], mounts[i], "none", 
MS_BIND|MS_REC|MS_RDONLY|MS_REMOUNT, NULL) < 0) {
 virReportSystemError(errno,
  _("Failed to make mount %s readonly"),
  mounts[i]);
@@ -994,7 +994,7 @@ static int lxcContainerMountBasicFS(bool userns_enabled,
 }
 
 if (bindOverReadonly &&
-mount(mnt_src, mnt->dst, NULL,
+mount(mnt_src, mnt->dst, "none",
   MS_BIND|MS_REMOUNT|mnt_mflags|MS_RDONLY, NULL) < 0) {
 virReportSystemError(errno,
  _("Failed to re-mount %s on %s flags=0x%x"),
@@ -1069,7 +1069,7 @@ static int lxcContainerMountFSDev(virDomainDefPtr def,
 VIR_DEBUG("Trying to %s %s to /dev", def->idmap.nuidmap ?
   "bind" : "move", path);
 
-if (mount(path, "/dev", NULL, flags, NULL) < 0) {
+if (mount(path, "/dev", "none", flags, NULL) < 0) {
 virReportSystemError(errno,
  _("Failed to mount %s on /dev"),
  path);
@@ -1105,7 +1105,7 @@ static int lxcContainerMountFSDevPTS(virDomainDefPtr def,
 VIR_DEBUG("Trying to %s %s to /dev/pts", def->idmap.nuidmap ?
   "bind" : "move", path);
 
-if (mount(path, "/dev/pts", NULL, flags, NULL) < 0) {
+if (mount(path, "/dev/pts", "none", flags, NULL) < 0) {
 virReportSystemError(errno,
  _("Failed to mount %s on /dev/pts"),
  path);
@@ -1215,7 +1215,7 @@ static int lxcContainerMountFSBind(virDomainFSDefPtr fs,
 }
 }
 
-if (mount(src, fs->dst, NULL, MS_BIND, NULL) < 0) {
+if (mount(src, fs->dst, "none", MS_BIND, NULL) < 0) {
 virReportSystemError(errno,
   

[libvirt] [PATCH 2/2] util: moving 'type' argument to avoid issues with mount() syscall.

[Julio Faracco]

This commit fixes a mount call inside virgroup.c file. The NULL value
into 'type' argument is causing a memory issue. See commit 794b576c2b
for more details. The best approach to fix it is moving NULL to "none"
filesytem.

Signed-off-by: Julio Faracco 
---
 src/util/vircgroup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 0a31947b0d..e810a3d81d 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -3962,7 +3962,7 @@ virCgroupBindMount(virCgroupPtr group, const char 
*oldroot,
 goto cleanup;
 }
 
-if (mount(src, group->controllers[i].mountPoint, NULL, MS_BIND,
+if (mount(src, group->controllers[i].mountPoint, "none", MS_BIND,
   NULL) < 0) {
 virReportSystemError(errno,
  _("Failed to bind cgroup '%s' on '%s'"),
-- 
2.17.1

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

[libvirt] [PATCH 0/2] {lxc, util}: Fixing issues with NULL argument for

[Julio Faracco]

This serie fixes the argument 'type' of syscall mount(). Valgrind is
throwing a memory issue when that parameter is NULL. The best option
to fix this issue is replace NULL to "none" filesystem.

Julio Faracco (2):
  lxc: moving 'type' argument to avoid issues with mount() syscall.
  util: moving 'type' argument to avoid issues with mount() syscall.

 src/lxc/lxc_container.c | 26 +-
 src/util/vircgroup.c|  2 +-
 2 files changed, 14 insertions(+), 14 deletions(-)

-- 
2.17.1

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

Re: [libvirt] [PATCH 1/8] snapshots: Avoid term 'checkpoint' for full system snapshot

[Eric Blake]

On 06/26/2018 10:56 AM, Nir Soffer wrote:

On Wed, Jun 13, 2018 at 7:42 PM Eric Blake  wrote:


Upcoming patches plan to introduce virDomainCheckpointPtr as a new
object for use in incremental backups, along with documentation
how incremental backups differ from snapshots.  But first, we need
to rename any existing mention of a 'system checkpoint' to instead
be a 'full system state snapshot', so that we aren't overloading
the term checkpoint.



I want to refer only to the new concept of checkpoint, compared with
snapshot.

I think checkpoint should refer to the current snapshot. When you perform
a backup, you should get the changed blocks in the current snapshot.


That is an incremental backup (copying only the blocks that have changed 
since some previous point in time) - and my design was that such points 
in time are named 'checkpoints', where the most recent checkpoint is the 
current checkpoint.  This is different from a snapshot (which is enough 
state that you can revert back to that point in time directly) - a 
checkpoint only carries enough information to perform an incremental 
backup rather than a rollback to earlier state.



When you restore, you want to the restore several complete snapshots,
and one partial snapshot, based on the backups of that snapshot.


I'm worried that overloading the term "snapshot" and/or "checkpoint" can 
make it difficult to see whether we are describing the same data motions.


You are correct that in order to roll a virtual machine back to state 
represented by a series of incremental backups will require 
reconstructing the state present on the machine at the desired point to 
roll back to.  But I'll have to read your example first to see if we're 
on the same page.




Lets try to see an example:

T1
- user create new vm marked for incremental backup
- system create base volume (S1)
- system create new dirty bitmap (B1)


Why do you need a dirty bitmap on a brand new system?  By definition, if 
the VM is brand new, every sector that the guest touches will be part of 
the first incremental backup, which is no different than taking a full 
backup of every sector?  But if it makes life easier by following 
consistent patterns, I also don't see a problem with creating a first 
checkpoint at the time an image is first created (my API proposal would 
allow you to create a domain, start it in the paused state, create a 
checkpoint, and then resume the guest so that it can start executing).




T2
- user create a snapshot
- dirty bitmap in original snapshot deactivated (B1)
- system create new snapshot (S2)
- system starts new dirty bitmap in the new snapshot (B2)


I'm still worried that interactions between snapshots (where the backing 
chain grows) and bitmaps may present interesting challenges.  But what 
you are describing here is that the act of creating a snapshot (to 
enlarge the backing chain) also has the effect of creating a snapshot (a 
new point in time for tracking incremental changes since the creation of 
the snapshot).  Whether we have to copy B1 into image S2, or whether 
image S2 can get by with just bitmap B2, is an implementation detail.




T3
- user create new checkpoint
- system deactivate current dirty bitmap (B2)
- system create new dirty bitmap (B3)
- user backups data in snapshot S2 using dirty bitmap B2
- user backups data in snapshot S1 using dirty bitmap B1


So here you are performing two incremental backups.  Note: the user can 
already backup S1 without using any new APIs, and without reference to 
bitmap B1 - that's because B1 was started when S1 was created, and 
closed out when S1 was no longer modified - but now that S1 is a 
read-only file in the backing chain, copying S1 is the same as copying 
the clusters covered by bitmap B1.


Also, my current API additions do NOT make it easy to grab just the 
incremental data covered by bitmap B1 at time T3; rather, the time to 
grab the copy of the data covered just by B1 is at time T2 when you 
create bitmap B2 (whether or not you also create file S2).  The API 
additions as I have proposed them only make it easy to grab a full 
backup of all data up to time T3 (no checkpoint as its start), an 
incremental backup of all data since T1 (checkpoint T1 as its start, 
using the merge of B1 and B2 to learn which clusters to grab), or an 
incremental backup of all data since T2 (checkpoint T2 as its start, 
using B2 to learn which clusters to grab).


If you NEED to grab an incremental snapshot whose history is NOT bounded 
by the current moment in time, then we need to rethink the operations we 
are offering via my new API.  On the bright side, since my API for 
virDomainBackupBegin() takes an XML description, we DO have the option 
of enhancing that XML to take a second point in time as the end boundary 
(it already has an optional  tag as the first point in time 
for the start boundary; or a full backup if that tag is omitted) - if we 
enhance that XML, we'd also have to figure out how to 

[libvirt] [PATCH] esx_driver: Simplify IsEncrypted and IsSecure

[Marcos Paulo de Souza]

Signed-off-by: Marcos Paulo de Souza 
---
 src/esx/esx_driver.c | 12 ++--
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 60aa5fc252..bb8eeecd42 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -4000,11 +4000,7 @@ esxConnectIsEncrypted(virConnectPtr conn)
 {
 esxPrivate *priv = conn->privateData;
 
-if (STRCASEEQ(priv->parsedUri->transport, "https")) {
-return 1;
-} else {
-return 0;
-}
+return STRCASEEQ(priv->parsedUri->transport, "https");
 }
 
 
@@ -4014,11 +4010,7 @@ esxConnectIsSecure(virConnectPtr conn)
 {
 esxPrivate *priv = conn->privateData;
 
-if (STRCASEEQ(priv->parsedUri->transport, "https")) {
-return 1;
-} else {
-return 0;
-}
+return STRCASEEQ(priv->parsedUri->transport, "https");
 }
 
 
-- 
2.17.1

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

[libvirt] [PATCH] syms: Fix placement of virDomainGetBlkioParametersAssignFromDef

[Cole Robinson]

It's in the domain_addr.h section, but should be in the
domain_conf.h section

Signed-off-by: Cole Robinson 
---
Pushed as trivial

 src/libvirt_private.syms | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f81333baf6..5499a368c0 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -110,7 +110,6 @@ virDomainCCWAddressReleaseAddr;
 virDomainCCWAddressSetCreate;
 virDomainCCWAddressSetFree;
 virDomainCCWAddressValidate;
-virDomainGetBlkioParametersAssignFromDef;
 virDomainPCIAddressAsString;
 virDomainPCIAddressBusIsFullyReserved;
 virDomainPCIAddressBusSetModel;
@@ -364,6 +363,7 @@ virDomainFSTypeToString;
 virDomainFSWrpolicyTypeFromString;
 virDomainFSWrpolicyTypeToString;
 virDomainGenerateMachineName;
+virDomainGetBlkioParametersAssignFromDef;
 virDomainGetFilesystemForTarget;
 virDomainGraphicsAuthConnectedTypeFromString;
 virDomainGraphicsAuthConnectedTypeToString;
-- 
2.17.1

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

Re: [libvirt] [PATCH 4/8] backup: Document new XML for backups

[Nir Soffer]

On Wed, Jun 13, 2018 at 7:42 PM Eric Blake  wrote:

> Prepare for new checkpoint and backup APIs by describing the XML
> that will represent a checkpoint.  This is modeled heavily after
> the XML for virDomainSnapshotPtr, since both represent a point in
> time of the guest.  But while a snapshot exists with the intent
> of rolling back to that state, a checkpoint instead makes it
> possible to create an incremental backup at a later time.
>
> Add testsuite coverage of a minimal use of the XML.
>
> Signed-off-by: Eric Blake 
> ---
>  docs/docs.html.in  |   3 +-
>  docs/domainstatecapture.html.in|   4 +-
>  docs/formatcheckpoint.html.in  | 273
> +
>  docs/schemas/domaincheckpoint.rng  |  89 ++
>  libvirt.spec.in|   1 +
>  mingw-libvirt.spec.in  |   2 +
>  tests/domaincheckpointxml2xmlin/empty.xml  |   1 +
>  tests/domaincheckpointxml2xmlout/empty.xml |  10 ++
>  tests/virschematest.c  |   2 +
>  9 files changed, 382 insertions(+), 3 deletions(-)
>  create mode 100644 docs/formatcheckpoint.html.in
>  create mode 100644 docs/schemas/domaincheckpoint.rng
>  create mode 100644 tests/domaincheckpointxml2xmlin/empty.xml
>  create mode 100644 tests/domaincheckpointxml2xmlout/empty.xml
>
> diff --git a/docs/docs.html.in b/docs/docs.html.in
> index 4c46b74980..11dfd27ba6 100644
> --- a/docs/docs.html.in
> +++ b/docs/docs.html.in
> @@ -79,7 +79,8 @@
>domain capabilities,
>node devices,
>secrets,
> -  snapshots
> +  snapshots,
> +  checkpoints
>
>  URI format
>  The URI formats used for connecting to libvirt
> diff --git a/docs/domainstatecapture.html.in b/docs/
> domainstatecapture.html.in
> index 00ab7e8ee1..4de93c87c8 100644
> --- a/docs/domainstatecapture.html.in
> +++ b/docs/domainstatecapture.html.in
> @@ -154,9 +154,9 @@
>  time as a new backup, so that the next incremental backup can
>  refer to the incremental state since the checkpoint created
>  during the current backup.  Guest state is then actually
> -captured using virDomainBackupBegin().  
> +this command.
>  
>
>  Examples
> diff --git a/docs/formatcheckpoint.html.in b/docs/formatcheckpoint.html.in
> new file mode 100644
> index 00..34507a9f68
> --- /dev/null
> +++ b/docs/formatcheckpoint.html.in
> @@ -0,0 +1,273 @@
> +
> +
> +http://www.w3.org/1999/xhtml;>
> +  
> +Checkpoint and Backup XML format
> +
> +
> +
> +Checkpoint XML
>

id=CheckpointXML?


> +
> +
> +  Domain disk backups, including incremental backups, are one form
> +  of domain state capture.
> +
> +
> +  Libvirt is able to facilitate incremental backups by tracking
> +  disk checkpoints, or points in time against which it is easy to
> +  compute which portion of the disk has changed.  Given a full
> +  backup (a backup created from the creation of the disk to a
> +  given point in time, coupled with the creation of a disk
> +  checkpoint at that time),


Not clear.


> and an incremental backup (a backup
> +  created from just the dirty portion of the disk between the
> +  first checkpoint and the second backup operation),


Also not clear.


> it is
> +  possible to do an offline reconstruction of the state of the
> +  disk at the time of the second backup, without having to copy as
> +  much data as a second full backup would require.  Most disk
> +  checkpoints are created in concert with a backup,
> +  via virDomainBackupBegin(); however, libvirt also
> +  exposes enough support to create disk checkpoints independently
> +  from a backup operation,
> +  via virDomainCheckpointCreateXML().
>

Thanks for the extra context.


> +
> +
> +  Attributes of libvirt checkpoints are stored as child elements of
> +  the domaincheckpoint element.  At checkpoint creation
> +  time, normally only the name, description,
> +  and disks elements are settable; the rest of the
> +  fields are ignored on creation, and will be filled in by
> +  libvirt in for informational purposes
>

So the user is responsible for creating checkpoints names? Do we use these
the same name in qcow2?


> +  by virDomainCheckpointGetXMLDesc().  However, when
> +  redefining a checkpoint,
> +  with the VIR_DOMAIN_CHECKPOINT_CREATE_REDEFINE flag
> +  of virDomainCheckpointCreateXML(), all of the XML
> +  described here is relevant.
> +
> +
> +  Checkpoints are maintained in a hierarchy.  A domain can have a
> +  current checkpoint, which is the most recent checkpoint compared to
> +  the current state of the domain (although a domain might have
> +  checkpoints without a current checkpoint, if checkpoints have been
> +  deleted in the meantime).  Creating or 

Re: [libvirt] [PATCH 3/8] backup: Introduce virDomainCheckpointPtr

[Nir Soffer]

On Wed, Jun 13, 2018 at 7:42 PM Eric Blake  wrote:

> Prepare for introducing a bunch of new public APIs related to
> backup checkpoints by first introducing a new internal type
> and errors associated with that type.  Checkpoints are modeled
> heavily after virDomainSnapshotPtr (both represent a point in
> time of the guest), although a snapshot exists with the intent
> of rolling back to that state, while a checkpoint exists to
> make it possible to create an incremental backup at a later
> time.
>
> Signed-off-by: Eric Blake 
> ---
>  include/libvirt/libvirt-domain-snapshot.h |  8 ++--
>  include/libvirt/libvirt.h |  2 +
>  include/libvirt/virterror.h   |  5 ++-
>  src/datatypes.c   | 62
> ++-
>  src/datatypes.h   | 31 +++-
>  src/libvirt_private.syms  |  2 +
>  src/util/virerror.c   | 15 +++-
>  7 files changed, 118 insertions(+), 7 deletions(-)
>
> diff --git a/include/libvirt/libvirt-domain-snapshot.h
> b/include/libvirt/libvirt-domain-snapshot.h
> index e5a893a767..ff1e890cfc 100644
> --- a/include/libvirt/libvirt-domain-snapshot.h
> +++ b/include/libvirt/libvirt-domain-snapshot.h
> @@ -31,15 +31,17 @@
>  /**
>   * virDomainSnapshot:
>   *
> - * a virDomainSnapshot is a private structure representing a snapshot of
> - * a domain.
> + * A virDomainSnapshot is a private structure representing a snapshot of
> + * a domain.  A snapshot captures the state of the domain at a point in
> + * time, with the intent that the guest can be reverted back to that
> + * state at a later time.
>

The extra context is very nice...


>   */
>  typedef struct _virDomainSnapshot virDomainSnapshot;
>
>  /**
>   * virDomainSnapshotPtr:
>   *
> - * a virDomainSnapshotPtr is pointer to a virDomainSnapshot private
> structure,
> + * A virDomainSnapshotPtr is pointer to a virDomainSnapshot private
> structure,
>   * and is the type used to reference a domain snapshot in the API.
>   */
>

But I think users of this API would like to find it here, explaining the
public
type.


>  typedef virDomainSnapshot *virDomainSnapshotPtr;
> diff --git a/include/libvirt/libvirt.h b/include/libvirt/libvirt.h
> index 36f6d60775..26887a40e7 100644
> --- a/include/libvirt/libvirt.h
> +++ b/include/libvirt/libvirt.h
> @@ -36,6 +36,8 @@ extern "C" {
>  # include 
>  # include 
>  # include 
> +typedef struct _virDomainCheckpoint virDomainCheckpoint;
> +typedef virDomainCheckpoint *virDomainCheckpointPtr;
>  # include 
>  # include 
>  # include 
> diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
> index 5e58b6a3f9..87ac16be0b 100644
> --- a/include/libvirt/virterror.h
> +++ b/include/libvirt/virterror.h
> @@ -4,7 +4,7 @@
>   * Description: Provides the interfaces of the libvirt library to handle
>   *  errors raised while using the library.
>   *
> - * Copyright (C) 2006-2016 Red Hat, Inc.
> + * Copyright (C) 2006-2018 Red Hat, Inc.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -133,6 +133,7 @@ typedef enum {
>  VIR_FROM_PERF = 65, /* Error from perf */
>  VIR_FROM_LIBSSH = 66,   /* Error from libssh connection transport
> */
>  VIR_FROM_RESCTRL = 67,  /* Error from resource control */
> +VIR_FROM_DOMAIN_CHECKPOINT = 68,/* Error from domain checkpoint */
>
>  # ifdef VIR_ENUM_SENTINELS
>  VIR_ERR_DOMAIN_LAST
> @@ -321,6 +322,8 @@ typedef enum {
> to guest-sync command
> (DEPRECATED)*/
>  VIR_ERR_LIBSSH = 98,/* error in libssh transport
> driver */
>  VIR_ERR_DEVICE_MISSING = 99,/* fail to find the desired
> device */
> +VIR_ERR_INVALID_DOMAIN_CHECKPOINT = 100,/* invalid domain checkpoint
> */
>

What is invalid checkpoint? It would be nice if there would not be
such thing.

Also the comment does not add anything.

+VIR_ERR_NO_DOMAIN_CHECKPOINT = 101, /* domain checkpoint not found */

 } virErrorNumber;
>
>  /**
> diff --git a/src/datatypes.c b/src/datatypes.c
> index 09b8eea5a2..3c9069c938 100644
> --- a/src/datatypes.c
> +++ b/src/datatypes.c
> @@ -1,7 +1,7 @@
>  /*
>   * datatypes.c: management of structs for public data types
>   *
> - * Copyright (C) 2006-2015 Red Hat, Inc.
> + * Copyright (C) 2006-2018 Red Hat, Inc.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -36,6 +36,7 @@ VIR_LOG_INIT("datatypes");
>  virClassPtr virConnectClass;
>  virClassPtr virConnectCloseCallbackDataClass;
>  virClassPtr virDomainClass;
> +virClassPtr virDomainCheckpointClass;
>  virClassPtr virDomainSnapshotClass;
>  virClassPtr virInterfaceClass;
>  virClassPtr virNetworkClass;
> @@ -49,6 +50,7 @@ virClassPtr virStoragePoolClass;
>  static void 

Re: [libvirt] [PATCH] qemu: hotplug: fix mdev attach for vfio-ccw

[John Ferlan]

On 06/26/2018 07:47 AM, Bjoern Walk wrote:
> Mediated devices of model 'vfio-ccw' are using CCW addresses, so make
> sure to call the correct address preparation code for the model.
> 
> Reviewed-by: Shalini Chellathurai Saroja 
> Reviewed-by: Boris Fiuczynski 
> Signed-off-by: Bjoern Walk 
> ---
>  src/qemu/qemu_hotplug.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 

Reviewed-by: John Ferlan 
(and pushed)

John

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

[libvirt] [PATCH] lxc: Rearrange order in lxcDomainUpdateDeviceFlags

[John Ferlan]

Although commit e3497f3f noted that the LIVE option doesn't
matter and removed the call to virDomainDefCompatibleDevice,
it didn't go quite far enough and change the order of the checks.

Since we don't have the possibility of LIVE succeeding and thus
the need for a delayed update in order to write the CONFIG change
let's just merge the saving of the config into one if statement.

Signed-off-by: John Ferlan 
---
 src/lxc/lxc_driver.c | 16 ++--
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index bde0ff6ad4..6afb36765a 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -4862,6 +4862,12 @@ static int lxcDomainUpdateDeviceFlags(virDomainPtr dom,
 goto endjob;
 }
 
+if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+   _("Unable to modify live devices"));
+goto endjob;
+}
+
 if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
 /* Make a copy for updated domain. */
 vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt);
@@ -4872,17 +4878,7 @@ static int lxcDomainUpdateDeviceFlags(virDomainPtr dom,
  * device we're going to update. */
 if ((ret = lxcDomainUpdateDeviceConfig(vmdef, dev)) < 0)
 goto endjob;
-}
-
-if (flags & VIR_DOMAIN_AFFECT_LIVE) {
-virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
-   _("Unable to modify live devices"));
-
-goto endjob;
-}
 
-/* Finally, if no error until here, we can save config. */
-if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
 ret = virDomainSaveConfig(cfg->configDir, driver->caps, vmdef);
 if (!ret) {
 virDomainObjAssignDef(vm, vmdef, false, NULL);
-- 
2.14.4

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

[libvirt] [PATCH] domain_addr: Fix weird comment format

[Cole Robinson]

Signed-off-by: Cole Robinson 
---
Pushed as trivial

 src/conf/domain_addr.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 8964973e03..39f22b82eb 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1348,12 +1348,12 @@ 
virDomainVirtioSerialAddrSetFree(virDomainVirtioSerialAddrSetPtr addrs)
 
 
 /* virDomainVirtioSerialAddrSetCreateFromDomain
-+ *
-+ * @def: Domain def to introspect
-+ *
-+ * Inspect the domain definition and return an address set containing
-+ * every virtio serial address we find
-+ */
+ *
+ * @def: Domain def to introspect
+ *
+ * Inspect the domain definition and return an address set containing
+ * every virtio serial address we find
+ */
 virDomainVirtioSerialAddrSetPtr
 virDomainVirtioSerialAddrSetCreateFromDomain(virDomainDefPtr def)
 {
-- 
2.17.1

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

Re: [libvirt] [PATCH v4 6/6] nwfilter: convert virt drivers to use public API for nwfilter bindings

[John Ferlan]

On 06/26/2018 06:23 AM, Daniel P. Berrangé wrote:
> Remove the callbacks that the nwfilter driver registers with the domain
> object config layer. Instead make the current helper methods call into
> the public API for creating/deleting nwfilter bindings.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/conf/domain_nwfilter.c | 135 +
>  src/conf/domain_nwfilter.h |  16 +--
>  src/libvirt_private.syms   |   1 -
>  src/lxc/lxc_process.c  |   2 +-
>  src/nwfilter/nwfilter_driver.c |  82 +++
>  src/nwfilter/nwfilter_gentech_driver.c |  42 
>  src/nwfilter/nwfilter_gentech_driver.h |   4 -
>  src/qemu/qemu_hotplug.c|   4 +-
>  src/qemu/qemu_interface.c  |   4 +-
>  src/qemu/qemu_process.c|   6 +-
>  src/remote/remote_daemon.c |   1 +
>  src/uml/uml_conf.c |   2 +-
>  12 files changed, 142 insertions(+), 157 deletions(-)
> 

My CPU's were kept quite busy during the avocado tests and the
environment is a bit fragile - causing a re-run because of bad user on
keyboard |-/. But I got through what was failing before, the aggressive
create/destroy filter while start/stop domain was successful unless it
was too busy smoking the CPU... Visually the code covers the issues I
was seeing before, so

Reviewed-by: John Ferlan 

John

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

Re: [libvirt] [PATCH 2/8] backup: Document nuances between different state capture APIs

[Nir Soffer]

On Wed, Jun 13, 2018 at 7:42 PM Eric Blake  wrote:

> Upcoming patches will add support for incremental backups via
> a new API; but first, we need a landing page that gives an
> overview of capturing various pieces of guest state, and which
> APIs are best suited to which tasks.


> Signed-off-by: Eric Blake 
> ---
>  docs/docs.html.in   |   5 ++
>  docs/domainstatecapture.html.in | 190
> 
>  docs/formatsnapshot.html.in |   2 +
>  3 files changed, 197 insertions(+)
>  create mode 100644 docs/domainstatecapture.html.in
>
> diff --git a/docs/docs.html.in b/docs/docs.html.in
> index 40e0e3b82e..4c46b74980 100644
> --- a/docs/docs.html.in
> +++ b/docs/docs.html.in
> @@ -120,6 +120,11 @@
>
>  Secure usage
>  Secure usage of the libvirt APIs
> +
> +Domain state
> +capture
> +Comparison between different methods of capturing domain
> +  state
>
>  
>
> diff --git a/docs/domainstatecapture.html.in b/docs/
> domainstatecapture.html.in
> new file mode 100644
> index 00..00ab7e8ee1
> --- /dev/null
> +++ b/docs/domainstatecapture.html.in
> @@ -0,0 +1,190 @@
> +
> +
> +http://www.w3.org/1999/xhtml;>
> +  
> +
> +Domain state capture using Libvirt
> +
> +
> +
> +
> +  This page compares the different means for capturing state
> +  related to a domain managed by libvirt, in order to aid
> +  application developers to choose which operations best suit
> +  their needs.
> +
> +
> +State capture trade-offs
> +
> +One of the features made possible with virtual machines is live
> +  migration, or transferring all state related to the guest from
> +  one host to another, with minimal interruption to the guest's
> +  activity.  A clever observer will then note that if all state is
> +  available for live migration, there is nothing stopping a user
> +  from saving that state at a given point of time, to be able to
> +  later rewind guest execution back to the state it previously
> +  had.  There are several different libvirt APIs associated with
> +  capturing the state of a guest, such that the captured state can
> +  later be used to rewind that guest to the conditions it was in
> +  earlier.  But since there are multiple APIs, it is best to
> +  understand the tradeoffs and differences between them, in order
> +  to choose the best API for a given task.
> +
> +
> +
> +  Timing
> +  Capturing state can be a lengthy process, so while the
> +captured state ideally represents an atomic point in time
> +correpsonding to something the guest was actually executing,
> +some interfaces require up-front preparation (the state
> +captured is not complete until the API ends, which may be some
> +time after the command was first started), while other
> +interfaces track the state when the command was first issued
> +even if it takes some time to finish capturing the state.
> +While it is possible to freeze guest I/O around either point
> +in time (so that the captured state is fully consistent,
> +rather than just crash-consistent), knowing whether the state
> +is captured at the start or end of the command may determine
> +which approach to use.  A related concept is the amount of
> +downtime the guest will experience during the capture,
> +particularly since freezing guest I/O has time
> +constraints.
> +
> +  Amount of state
> +  For an offline guest, only the contents of the guest disks
> +needs to be captured; restoring that state is merely a fresh
> +boot with the disks restored to that state.  But for an online
> +guest, there is a choice between storing the guest's memory
> +(all that is needed during live migration where the storage is
> +shared between source and destination), the guest's disk state
> +(all that is needed if there are no pending guest I/O
> +transactions that would be lost without the corresponding
> +memory state), or both together.  Unless guest I/O is quiesced
> +prior to capturing state, then reverting to captured disk
> +state of a live guest without the corresponding memory state
> +is comparable to booting a machine that previously lost power
> +without a clean shutdown; but for a guest that uses
> +appropriate journaling methods, this crash-consistent state
> +may be sufficient to avoid the additional storage and time
> +needed to capture memory state.
> +
> +  Quantity of files
> +  When capturing state, some approaches store all state within
> +the same file (internal), while others expand a chain of
> +related files that must be used together (external), for more
> +files that a management 

Re: [libvirt] [PATCH 1/8] snapshots: Avoid term 'checkpoint' for full system snapshot

[Nir Soffer]

On Wed, Jun 13, 2018 at 7:42 PM Eric Blake  wrote:

> Upcoming patches plan to introduce virDomainCheckpointPtr as a new
> object for use in incremental backups, along with documentation
> how incremental backups differ from snapshots.  But first, we need
> to rename any existing mention of a 'system checkpoint' to instead
> be a 'full system state snapshot', so that we aren't overloading
> the term checkpoint.
>

I want to refer only to the new concept of checkpoint, compared with
snapshot.

I think checkpoint should refer to the current snapshot. When you perform
a backup, you should get the changed blocks in the current snapshot.
When you restore, you want to the restore several complete snapshots,
and one partial snapshot, based on the backups of that snapshot.

Lets try to see an example:

T1
- user create new vm marked for incremental backup
- system create base volume (S1)
- system create new dirty bitmap (B1)

T2
- user create a snapshot
- dirty bitmap in original snapshot deactivated (B1)
- system create new snapshot (S2)
- system starts new dirty bitmap in the new snapshot (B2)

T3
- user create new checkpoint
- system deactivate current dirty bitmap (B2)
- system create new dirty bitmap (B3)
- user backups data in snapshot S2 using dirty bitmap B2
- user backups data in snapshot S1 using dirty bitmap B1

T4
- user create new checkpoint
- system deactivate current dirty bitmap (B3)
- system create new dirty bitmap (B4)
- user backups data in snapshot S2 using dirty bitmap B3

Lets say use want to restore to state as it was in T3

This is the data kept by the backup application:

- snapshots
  - S1
- checkpoints
  - B1
  - S2
- checkpoints
  - B2
  - B3

T5
- user start restore to state in time T3
- user create new disk
- user create empty snapshot S1
- user upload snapshot S1 data to stoage
- user create empty snaphost disk S2
- user upload snapshot S1 data to stoage

John, are dirty bitmaps implemented in this way in qemu?

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

Re: [libvirt] [PATCH v2 2/3] conf: Reintroduce action to virDomainDefCompatibleDevice

[John Ferlan]

On 06/26/2018 11:35 AM, Michal Privoznik wrote:
> On 06/26/2018 04:30 PM, John Ferlan wrote:
>>
>>
>> On 06/26/2018 06:21 AM, Michal Privoznik wrote:
>>> This was lost in c57f3fd2f8999d17e01. But now we are going to
>>> need it again.
>>
>> Looks like that commit also "lost" the compatible device check on detach
>> too (e.g. ACTION_DETACH) without explanation.  Whether at the time it
>> then became the last consumer of virDomainDeviceAction is another question.
> 
> There's no need to check for device compatibility on device detach, is
> there? The virDomainDefCompatibleDevice() merely checks whether new
> device that we are trying to hot-plug or change would not break
> assumptions made by other parts of domain config. These can't be broken
> if we're removing a device.
> 

Well it was there at some point in time and it's removal (or reason
therein) wasn't provided in either the bz or the commit message from the
referenced patch. So that led me down the path of curiosity.

Maybe use this opportunity to add to the commit message that restoring
the DETACH checks isn't necessary for the reasons you've outlined
although it's not that important.

>>
>> So, since you're bringing this back to life - should those checks be
>> reinstated into the Detach code (lxc, qemu)? If not, surely
>> ACTION_DETACH then still isn't used.
> 
> Yes, it is unused. I can remove it, although for completion sake I
> rather have it in.
> 

No need to remove it now, but since it's not used, perhaps eventually
some trivial patch will wind it's way through the system.  When that
patch makes it's way through maybe it can have the explanation for why
DETACH checking wasn't necessary.

>>
>> Can I assume you tested bz1439991 with the new condition?
>>
> 
> Yes you can. I've tested this.
> 
> Michal
> 

OK, so considered this patch,

Reviewed-by: John Ferlan 

John

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

Re: [libvirt] [PATCH python] libvirtaio: Fix compat with python 3.7

[Andrea Bolognani]

On Mon, 2018-06-25 at 14:35 -0400, Cole Robinson wrote:
> If we don't care about python3 < 3.4.4 compat, we can just do
> 
>   from asyncio import ensure_future

Debian 8, which is still a supported target platform[1], only
has Python 3.4.2.


[1] At least for libvirt; I'm going to assume libvirt-python
doesn't have its own support policy.
-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH v4 2/6] nwfilter: keep track of active filter bindings

[John Ferlan]

On 06/26/2018 06:23 AM, Daniel P. Berrangé wrote:
> Currently the nwfilter driver does not keep any record of what filter
> bindings it has active. This means that when it needs to recreate
> filters, it has to rely on triggering callbacks provided by the virt
> drivers. This introduces a hash table recording the virNWFilterBinding
> objects so the driver has a record of all active filters.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  src/conf/virnwfilterobj.h  |  4 ++
>  src/nwfilter/nwfilter_driver.c | 84 --
>  2 files changed, 63 insertions(+), 25 deletions(-)
> 

Reviewed-by: John Ferlan 

John

BTW: I'm just running the avocado test again for final patch 6/6 checks.

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

Re: [libvirt] [PATCH v2 3/3] conf: Forbid device alias change on device-update

[Michal Privoznik]

On 06/26/2018 05:04 PM, John Ferlan wrote:
> 
> 
> On 06/26/2018 06:21 AM, Michal Privoznik wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1585108
>>
>> When updating a live device users might pass different alias than
>> the one the device has. Currently, this is silently ignored which
>> goes against our behaviour for other parts of the device where we
>> explicitly allow only certain changes and error out loudly on
>> anything else.
>>
>> Signed-off-by: Michal Privoznik 
>> ---
>>  src/conf/domain_conf.c  | 13 -
>>  src/conf/domain_conf.h  |  3 ++-
>>  src/lxc/lxc_driver.c|  9 ++---
>>  src/qemu/qemu_domain.c  |  8 
>>  src/qemu/qemu_driver.c  | 24 
>>  src/qemu/qemu_hotplug.c |  5 -
>>  6 files changed, 36 insertions(+), 26 deletions(-)
>>
> 
> Reviewed-by: John Ferlan 
> 
> John
> 
> NB: I've left a couple of "my review notes" below for your consideration
> or thoughts... They are somewhat tangents to the changes, but may give
> you reason to add a comment or two somewhere if you feel compelled to do so.
> 
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 93cfca351c..b8b53450fa 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -28206,7 +28206,8 @@ int
>>  virDomainDefCompatibleDevice(virDomainDefPtr def,
>>   virDomainDeviceDefPtr dev,
>>   virDomainDeviceDefPtr oldDev,
>> - virDomainDeviceAction action ATTRIBUTE_UNUSED)
>> + virDomainDeviceAction action,
>> + bool live)
>>  {
>>  virDomainCompatibleDeviceData data = {
>>  .newInfo = virDomainDeviceGetInfo(dev),
>> @@ -28216,6 +28217,16 @@ virDomainDefCompatibleDevice(virDomainDefPtr def,
>>  if (oldDev)
>>  data.oldInfo = virDomainDeviceGetInfo(oldDev);
>>  
>> +if (action == VIR_DOMAIN_DEVICE_ACTION_UPDATE &&
>> +live &&
>> +((!!data.newInfo != !!data.oldInfo) ||
>> + (data.newInfo && data.oldInfo &&
>> +  STRNEQ_NULLABLE(data.newInfo->alias, data.oldInfo->alias {
>> +virReportError(VIR_ERR_OPERATION_DENIED, "%s",
>> +   _("changing device alias is not allowed"));
>> +return -1;
>> +}
>> +
> 
> I'd assume that this isn't affected by whether or not REMOVE was
> supplied since we seem to allow a lot less to match for REMOVE and I
> wouldn't think alias really matters at that point.

Yes. Users are required to pass full device XML for detach. To cite from
virDomainDetachDeviceFlags() documentation:

  The supplied XML description of the device should be as specific
  as its definition in the domain XML. The set of attributes used
  to match the device are internal to the drivers. Using a partial definition,
  or attempting to detach a device that is not present in the domain XML,
  but shares some specific attributes with one that is present,
  may lead to unexpected results.

So at REMOVE it doesn't really matter if alias matches or not. We can
change the code to require matching alias, but I think that is another
issue orthogonal to this one ;-)

> 
> I assume what happens is people use the same XML for attach as they use
> for remove and thus don't provide alias for either.  Update - yeah,
> that's a different scenario.

Yep, because of the reasoning I provided in one of the replies to
previous version. Users provide us with new device XML and any
difference to old XML must be treated as request for change. But we
can't change some things for live XML (e.g. alias), therefore we have to
check for them explicitly and error out.

> 
>>  if (!virDomainDefHasUSB(def) &&
>>  def->os.type != VIR_DOMAIN_OSTYPE_EXE &&
>>  virDomainDeviceIsUSB(dev)) {
> 
> [...]
> 
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index 6d203e1f2e..d750f3382a 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -8613,14 +8613,6 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr 
>> disk,
>>  return false;
>>  }
>>  
> 
> Perhaps leaving the breadcrumb (e.g. comment) here or at the top of the
> function indicating alias is checked in virDomainDefCompatibleDevice.
> That then at least "matches" what the function description indicates and
> the reason why alias isn't specifically checked.

Okay, done.

> 
>> -if (disk->info.alias &&
>> -STRNEQ_NULLABLE(disk->info.alias, orig_disk->info.alias)) {
>> -virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
>> -   _("cannot modify field '%s' of the disk"),
>> -   "alias");
>> -return false;
>> -}
>> -
>>  CHECK_EQ(info.bootIndex, "boot order", true);
>>  CHECK_EQ(rawio, "rawio", true);
>>  CHECK_EQ(sgio, "sgio", true);
> 
> [...]
> 
>>  
>>  if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, driver)) < 0)
>> diff --git 

Re: [libvirt] [PATCH v4 1/6] virsh: add manpage docs for nwfilter-binding commands.

[John Ferlan]

On 06/26/2018 06:23 AM, Daniel P. Berrangé wrote:
> Signed-off-by: Daniel P. Berrangé 
> ---
>  tools/virsh.pod | 35 +++
>  1 file changed, 35 insertions(+)
> 

Just need to add periods to the end of a couple of sentences below

Reviewed-by: John Ferlan 

John

> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index c9ef4f137c..47985ebf78 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -4807,6 +4807,41 @@ variables, and defaults to C.
>  
>  =back
>  
> +=head1 NWFILTER BINDING COMMANDS
> +
> +The following commands manipulate network filter bindings. Network filter
> +bindings track the association between a network port and a network
> +filter. Generally the bindings are managed automatically by the hypervisor
> +drivers when adding/removing NICs on a guest.
> +
> +If an admin is creating/deleting TAP devices for non-guest usage,
> +however, the network filter binding commands provide a way to make use
> +of the network filters directly.
> +
> +=over 4
> +
> +=item B I
> +
> +Associate a network port with a network filter. The network filter backend
> +will immediately attempt to instantiate the filter rules on the port.
> +
> +=item B I
> +
> +Disassociate a network port from a network filter. The network filter
> +backend will immediately tear down the filter rules that exist on the
> +port.
> +
> +=item B
> +
> +List all of the network ports which have filters associated with them

s/them/them.

> +
> +=item B I
> +
> +Output the network filter binding XML for the network device called
> +C

s/>/>.

> +
> +=back
> +
>  =head1 HYPERVISOR-SPECIFIC COMMANDS
>  
>  NOTE: Use of the following commands is B discouraged.  They
> 

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

Re: [libvirt] [PATCH v2 10/11] tests/qemuxml2argvtest: test RISC-V virt machine arguments

[Andrea Bolognani]

On Thu, 2018-06-14 at 22:32 +0200, Lubomir Rintel wrote:
[...]
> +++ b/tests/qemuxml2argvdata/riscv64-virt.xml
> @@ -0,0 +1,32 @@
> +
> +  riscv64
> +  fd65fc03-8838-4c4d-9d8d-395802488790

A lot of the information you included in this file can be omitted,
and doing so not only highlights which parts actually matter, but
also ensures that libvirt is able to fill in the blanks on its own
as you would expect it to.

  
riscv64
fd65fc03-8838-4c4d-9d8d-395802488790
2097152
1

  hvm
  /var/lib/libvirt/images/bbl
  console=ttyS0 ro root=/dev/vda


  /usr/bin/qemu-system-riscv64
  


  
  

  

The above is all you need.

[...]
> +++ b/tests/qemuxml2argvtest.c
> @@ -2903,6 +2903,9 @@ mymain(void)
>  QEMU_CAPS_KVM,
>  QEMU_CAPS_SEV_GUEST);
>  
> +DO_TEST("riscv64-virt",
> +QEMU_CAPS_DEVICE_VIRTIO_MMIO);

You forgot to add the same snippet to tests/qemuxml2xmltest.c, and
without that XML round-trip will not actually be tested :)


With the above addressed,

  Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH v2 2/3] conf: Reintroduce action to virDomainDefCompatibleDevice

[Michal Privoznik]

On 06/26/2018 04:30 PM, John Ferlan wrote:
> 
> 
> On 06/26/2018 06:21 AM, Michal Privoznik wrote:
>> This was lost in c57f3fd2f8999d17e01. But now we are going to
>> need it again.
> 
> Looks like that commit also "lost" the compatible device check on detach
> too (e.g. ACTION_DETACH) without explanation.  Whether at the time it
> then became the last consumer of virDomainDeviceAction is another question.

There's no need to check for device compatibility on device detach, is
there? The virDomainDefCompatibleDevice() merely checks whether new
device that we are trying to hot-plug or change would not break
assumptions made by other parts of domain config. These can't be broken
if we're removing a device.

> 
> So, since you're bringing this back to life - should those checks be
> reinstated into the Detach code (lxc, qemu)? If not, surely
> ACTION_DETACH then still isn't used.

Yes, it is unused. I can remove it, although for completion sake I
rather have it in.

> 
> Can I assume you tested bz1439991 with the new condition?
> 

Yes you can. I've tested this.

Michal

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

Re: [libvirt] [PATCH v2 06/11] qemu: assign addresses to virtio devices on RISC-V

[Andrea Bolognani]

On Thu, 2018-06-14 at 22:32 +0200, Lubomir Rintel wrote:
[...]
> +static void
> +qemuDomainAssignRISCVVirtioMMIOAddresses(virDomainDefPtr def,
> + virQEMUCapsPtr qemuCaps)
> +{
> +if (!ARCH_IS_RISCV(def->os.arch))
> +return;
> +
> +if (STRNEQ(def->os.machine, "virt"))
> +return;

This will stop working the moment QEMU introduces versioned
machine types for RISC-V, which makes it not very future proof.

You should instead introduce (in a separate, earlier patch)

  qemuDomainIsRISCVVirt()
  qemuDomainMachineIsRISCVVirt()

functions modeled after the Arm equivalent, and use the former
here.

[...]
> @@ -2940,6 +2957,8 @@ qemuDomainAssignAddresses(virDomainDefPtr def,
>  
>  qemuDomainAssignARMVirtioMMIOAddresses(def, qemuCaps);
>  
> +qemuDomainAssignRISCVVirtioMMIOAddresses(def, qemuCaps);

We should have a

  qemuDomainAssignVirtioMMIOAddresses()

wrapper that calls both the Arm and RISC-V variants, and call
that one from qemuDomainAssignAddresses().

You can introduce and start using the wrapper in a separate patch;
then, in this one, you can add the RISC-V variant and modify the
wrapper so that it calls it in addition to the Arm variant.

Everything else looks good.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH v2 3/3] conf: Forbid device alias change on device-update

[John Ferlan]

On 06/26/2018 06:21 AM, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1585108
> 
> When updating a live device users might pass different alias than
> the one the device has. Currently, this is silently ignored which
> goes against our behaviour for other parts of the device where we
> explicitly allow only certain changes and error out loudly on
> anything else.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/conf/domain_conf.c  | 13 -
>  src/conf/domain_conf.h  |  3 ++-
>  src/lxc/lxc_driver.c|  9 ++---
>  src/qemu/qemu_domain.c  |  8 
>  src/qemu/qemu_driver.c  | 24 
>  src/qemu/qemu_hotplug.c |  5 -
>  6 files changed, 36 insertions(+), 26 deletions(-)
> 

Reviewed-by: John Ferlan 

John

NB: I've left a couple of "my review notes" below for your consideration
or thoughts... They are somewhat tangents to the changes, but may give
you reason to add a comment or two somewhere if you feel compelled to do so.

> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 93cfca351c..b8b53450fa 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -28206,7 +28206,8 @@ int
>  virDomainDefCompatibleDevice(virDomainDefPtr def,
>   virDomainDeviceDefPtr dev,
>   virDomainDeviceDefPtr oldDev,
> - virDomainDeviceAction action ATTRIBUTE_UNUSED)
> + virDomainDeviceAction action,
> + bool live)
>  {
>  virDomainCompatibleDeviceData data = {
>  .newInfo = virDomainDeviceGetInfo(dev),
> @@ -28216,6 +28217,16 @@ virDomainDefCompatibleDevice(virDomainDefPtr def,
>  if (oldDev)
>  data.oldInfo = virDomainDeviceGetInfo(oldDev);
>  
> +if (action == VIR_DOMAIN_DEVICE_ACTION_UPDATE &&
> +live &&
> +((!!data.newInfo != !!data.oldInfo) ||
> + (data.newInfo && data.oldInfo &&
> +  STRNEQ_NULLABLE(data.newInfo->alias, data.oldInfo->alias {
> +virReportError(VIR_ERR_OPERATION_DENIED, "%s",
> +   _("changing device alias is not allowed"));
> +return -1;
> +}
> +

I'd assume that this isn't affected by whether or not REMOVE was
supplied since we seem to allow a lot less to match for REMOVE and I
wouldn't think alias really matters at that point.

I assume what happens is people use the same XML for attach as they use
for remove and thus don't provide alias for either.  Update - yeah,
that's a different scenario.

>  if (!virDomainDefHasUSB(def) &&
>  def->os.type != VIR_DOMAIN_OSTYPE_EXE &&
>  virDomainDeviceIsUSB(dev)) {

[...]

> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 6d203e1f2e..d750f3382a 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -8613,14 +8613,6 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
>  return false;
>  }
>  

Perhaps leaving the breadcrumb (e.g. comment) here or at the top of the
function indicating alias is checked in virDomainDefCompatibleDevice.
That then at least "matches" what the function description indicates and
the reason why alias isn't specifically checked.

> -if (disk->info.alias &&
> -STRNEQ_NULLABLE(disk->info.alias, orig_disk->info.alias)) {
> -virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> -   _("cannot modify field '%s' of the disk"),
> -   "alias");
> -return false;
> -}
> -
>  CHECK_EQ(info.bootIndex, "boot order", true);
>  CHECK_EQ(rawio, "rawio", true);
>  CHECK_EQ(sgio, "sgio", true);

[...]

>  
>  if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, driver)) < 0)
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 7a1bbc7c8c..a8991800b3 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -3193,11 +3193,6 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
>  if (!newdev->info.alias &&
>  VIR_STRDUP(newdev->info.alias, olddev->info.alias) < 0)
>  goto cleanup;

I'll casually note that based on what I showed from the last review the
the comment at the top of virDomainDeviceInfoCopy doesn't seem to ring
true and thus if virDomainDeviceAddressIsValid does return 0 (for
multiple reasons) it would cause virDomainDeviceInfoCopy to overwrite
alias, romfile, and loadparm.

But that's unrelated to your code.

This at least takes care of the empty alias case, which is expected.
Whether you decide to note that virDomainDefCompatibleDevice ensures the
provided alias matches - is your call.  Since there is only one caller
to this function, removing this check is fine because of that call
(hence the hedge whether to note it or not so that some future change
realizes that alias is checked elsewhere).

John

> -if (STRNEQ_NULLABLE(olddev->info.alias, newdev->info.alias)) {
> -

Re: [libvirt] [PATCH v2 05/11] qemu: no USB by default on RISC-V machines

[Andrea Bolognani]

On Thu, 2018-06-14 at 22:32 +0200, Lubomir Rintel wrote:
> Signed-off-by: Lubomir Rintel 
> ---
>  src/qemu/qemu_domain.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 275090fb58..72b72364d5 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -3198,6 +3198,11 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
>  addPCIRoot = true;
>  break;
>  
> +case VIR_ARCH_RISCV32:
> +case VIR_ARCH_RISCV64:
> +addDefaultUSB = false;
> +break;
> +
>  case VIR_ARCH_S390:
>  case VIR_ARCH_S390X:
>  addDefaultUSB = false;
> @@ -3224,8 +3229,6 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def,
>  case VIR_ARCH_OR32:
>  case VIR_ARCH_PARISC:
>  case VIR_ARCH_PARISC64:
> -case VIR_ARCH_RISCV32:
> -case VIR_ARCH_RISCV64:
>  case VIR_ARCH_PPCLE:
>  case VIR_ARCH_UNICORE32:
>  case VIR_ARCH_XTENSA:

Reviewed-by: Andrea Bolognani 

I forgot to mention this earlier, but this patch should go after
the ones (actually one, due to squashing :) that add RISC-V
architecture support to the test suite.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH v2 2/3] conf: Reintroduce action to virDomainDefCompatibleDevice

[John Ferlan]

On 06/26/2018 06:21 AM, Michal Privoznik wrote:
> This was lost in c57f3fd2f8999d17e01. But now we are going to
> need it again.

Looks like that commit also "lost" the compatible device check on detach
too (e.g. ACTION_DETACH) without explanation.  Whether at the time it
then became the last consumer of virDomainDeviceAction is another question.

So, since you're bringing this back to life - should those checks be
reinstated into the Detach code (lxc, qemu)? If not, surely
ACTION_DETACH then still isn't used.

Can I assume you tested bz1439991 with the new condition?

The restoration of what's been done thus far looks fine, just need some
more details...

John

> 
> Signed-off-by: Michal Privoznik 
> ---
>  src/conf/domain_conf.c |  3 ++-
>  src/conf/domain_conf.h |  3 ++-
>  src/lxc/lxc_driver.c   |  9 ++---
>  src/qemu/qemu_driver.c | 24 
>  4 files changed, 26 insertions(+), 13 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index d8cb7f37f3..93cfca351c 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -28205,7 +28205,8 @@ virDomainDeviceInfoCheckBootIndex(virDomainDefPtr def 
> ATTRIBUTE_UNUSED,
>  int
>  virDomainDefCompatibleDevice(virDomainDefPtr def,
>   virDomainDeviceDefPtr dev,
> - virDomainDeviceDefPtr oldDev)
> + virDomainDeviceDefPtr oldDev,
> + virDomainDeviceAction action ATTRIBUTE_UNUSED)
>  {
>  virDomainCompatibleDeviceData data = {
>  .newInfo = virDomainDeviceGetInfo(dev),
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 0924fc4f3c..f33405e097 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -3106,7 +3106,8 @@ typedef enum {
>  
>  int virDomainDefCompatibleDevice(virDomainDefPtr def,
>   virDomainDeviceDefPtr dev,
> - virDomainDeviceDefPtr oldDev);
> + virDomainDeviceDefPtr oldDev,
> + virDomainDeviceAction action);
>  
>  void virDomainRNGDefFree(virDomainRNGDefPtr def);
>  
> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index cfb431488d..850b12726b 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -3549,7 +3549,8 @@ lxcDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
>  goto cleanup;
>  
>  oldDev.data.net = vmdef->nets[idx];
> -if (virDomainDefCompatibleDevice(vmdef, dev, ) < 0)
> +if (virDomainDefCompatibleDevice(vmdef, dev, ,
> + VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 
> 0)
>  return -1;
>  
>  virDomainNetDefFree(vmdef->nets[idx]);
> @@ -4785,7 +4786,8 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom,
>  if (!vmdef)
>  goto endjob;
>  
> -if (virDomainDefCompatibleDevice(vmdef, dev, NULL) < 0)
> +if (virDomainDefCompatibleDevice(vmdef, dev, NULL,
> + VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 
> 0)
>  goto endjob;
>  
>  if ((ret = lxcDomainAttachDeviceConfig(vmdef, dev)) < 0)
> @@ -4793,7 +4795,8 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom,
>  }
>  
>  if (flags & VIR_DOMAIN_AFFECT_LIVE) {
> -if (virDomainDefCompatibleDevice(vm->def, dev_copy, NULL) < 0)
> +if (virDomainDefCompatibleDevice(vm->def, dev_copy, NULL,
> + VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 
> 0)
>  goto endjob;
>  
>  if ((ret = lxcDomainAttachDeviceLive(dom->conn, driver, vm, 
> dev_copy)) < 0)
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index e3fb4919a5..8c0c681c4d 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7885,7 +7885,8 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm,
>  }
>  
>  oldDev.data.disk = orig_disk;
> -if (virDomainDefCompatibleDevice(vm->def, dev, ) < 0)
> +if (virDomainDefCompatibleDevice(vm->def, dev, ,
> + VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0)
>  goto cleanup;
>  
>  if (!qemuDomainDiskChangeSupported(disk, orig_disk))
> @@ -7943,7 +7944,8 @@ qemuDomainUpdateDeviceLive(virDomainObjPtr vm,
>  case VIR_DOMAIN_DEVICE_GRAPHICS:
>  if ((idx = qemuDomainFindGraphicsIndex(vm->def, dev->data.graphics)) 
> >= 0) {
>  oldDev.data.graphics = vm->def->graphics[idx];
> -if (virDomainDefCompatibleDevice(vm->def, dev, ) < 0)
> +if (virDomainDefCompatibleDevice(vm->def, dev, ,
> + 
> VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0)
>  return -1;
>  }
>  
> @@ -7953,7 +7955,8 @@ qemuDomainUpdateDeviceLive(virDomainObjPtr vm,
>  case VIR_DOMAIN_DEVICE_NET:
>  if ((idx = 

Re: [libvirt] [PATCH python] libvirtaio: Fix compat with python 3.7

[Cole Robinson]

On 06/25/2018 02:35 PM, Cole Robinson wrote:
> In python 3.7, async is now a keyword, so this throws a syntax error:
> 
>   File "/usr/lib64/python3.7/site-packages/libvirtaio.py", line 49
> from asyncio import async as ensure_future
> ^
>   SyntaxError: invalid syntax
> 
> Switch to getattr trickery to accomplish the same goal
> 
> Signed-off-by: Cole Robinson 
> ---
>  libvirtaio.py | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> If we don't care about python3 < 3.4.4 compat, we can just do
> 
>   from asyncio import ensure_future
> 

Also I meant to say, this was prompted by python3.7 rebase in Fedora:

https://lists.fedoraproject.org/archives/list/de...@lists.fedoraproject.org/message/5DOPLN7K7GCI3T6ZMXDBIPJW5Y7ESGYS/

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

Re: [libvirt] [PATCH v2 1/3] qemuDomainUpdateDeviceFlags: Parse device as live if needed

[John Ferlan]

On 06/26/2018 06:21 AM, Michal Privoznik wrote:
> When updating device it's worth parsing live info too as users
> might want to update it as well.
> 
> Signed-off-by: Michal Privoznik 
> 
> Reviewed-by: John Ferlan 
> Signed-off-by: Michal Privoznik 

^^ Clean this up - I assume you added the R-by and the extra which
caused your automatic one to be confused and thus add another for the
send - just check to see if it's clean in whatever is pushed...

John

> ---
>  src/qemu/qemu_driver.c | 12 
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 

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

Re: [libvirt] [PATCH v2 11/11] tests/virschematest: add RISC-V to capabilityschemadata/caps-qemu-kvm

[Andrea Bolognani]

On Thu, 2018-06-14 at 22:32 +0200, Lubomir Rintel wrote:
> Signed-off-by: Lubomir Rintel 
> ---
>  tests/capabilityschemadata/caps-qemu-kvm.xml | 36 
>  1 file changed, 36 insertions(+)

Looks good, just squash it into 07/11 along with the rest.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH v2 09/11] tests/captest: check RISC-V capabilities lookup

[Andrea Bolognani]

On Thu, 2018-06-14 at 22:32 +0200, Lubomir Rintel wrote:
> Signed-off-by: Lubomir Rintel 
> ---
>  tests/vircapstest.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/tests/vircapstest.c b/tests/vircapstest.c
> index 1df3fa091f..e016e466d2 100644
> --- a/tests/vircapstest.c
> +++ b/tests/vircapstest.c
> @@ -180,6 +180,12 @@ test_virCapsDomainDataLookupQEMU(const void *data 
> ATTRIBUTE_UNUSED)
>  CAPSCOMP(-1, VIR_ARCH_NONE, VIR_DOMAIN_VIRT_NONE, 
> "/usr/bin/qemu-system-ppc64", NULL,
>  VIR_DOMAIN_OSTYPE_HVM, VIR_ARCH_PPC64,
>  VIR_DOMAIN_VIRT_QEMU, "/usr/bin/qemu-system-ppc64", "pseries");
> +CAPSCOMP(-1, VIR_ARCH_RISCV32, VIR_DOMAIN_VIRT_NONE, NULL, NULL,
> +VIR_DOMAIN_OSTYPE_HVM, VIR_ARCH_RISCV32,
> +VIR_DOMAIN_VIRT_QEMU, "/usr/bin/qemu-system-riscv32", "spike_v1.10");
> +CAPSCOMP(-1, VIR_ARCH_RISCV64, VIR_DOMAIN_VIRT_NONE, NULL, NULL,
> +VIR_DOMAIN_OSTYPE_HVM, VIR_ARCH_RISCV64,
> +VIR_DOMAIN_VIRT_QEMU, "/usr/bin/qemu-system-riscv64", "spike_v1.10");

So spike_v1.10 is the default machine type, uh?

That's a bummer, I would very much have preferred an Arm-style
situation where there is no default and you have to be explicit
instead... I guess that's out of our hands though :)

Looks good, just squash this one into 07/11 as well.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH v2 08/11] tests/util: add RISC-V capabilities

[Andrea Bolognani]

On Thu, 2018-06-14 at 22:32 +0200, Lubomir Rintel wrote:
> +static int testQemuAddRISCV32Guest(virCapsPtr caps)
> +{
> +static const char *machine[] = { "spike_v1.10",
> + "spike_v1.9.1",
> + "sifive_e",
> + "virt",
> + "sifive_u" };
> +virCapsGuestMachinePtr *machines = NULL;
> +virCapsGuestPtr guest;
> +
> +machines = virCapabilitiesAllocMachines(machine, 1);

This is wrong: the second argument to the function is the number
of machines you're adding, so it should look like

  virCapabilitiesAllocMachines(machine,
   ARRAY_CARDINALITY(machine))

instead. While you're at it, you can give the variable a better
name, like for example... 'names' :)

Actually, since you're going to need that value more than once,
you will probably want to introduce

  static const int nmachines = ARRAY_CARDINALITY(names);

and just use 'nmachines' in the call to AllocMachines() above...

> +if (!machines)
> +goto error;
> +
> +guest = virCapabilitiesAddGuest(caps, VIR_DOMAIN_OSTYPE_HVM, 
> VIR_ARCH_RISCV32,
> +QEMUBinList[TEST_UTILS_QEMU_BIN_RISCV32],
> +NULL, 1, machines);

... as well as here...

> +if (!guest)
> +goto error;
> +
> +if (!virCapabilitiesAddGuestDomain(guest, VIR_DOMAIN_VIRT_QEMU, NULL, 
> NULL, 0, NULL))
> +goto error;
> +
> +return 0;
> +
> + error:
> +/* No way to free a guest? */
> +virCapabilitiesFreeMachines(machines, 1);

... and here.

(Drop the comment here as well.)

Looking at this also made me realize there are several existing
instances of it not being handled correctly; I'll take care of
fixing them.

[...]
> @@ -422,6 +488,12 @@ virCapsPtr testQemuCapsInit(void)
>  if (testQemuAddPPCGuest(caps))
>  goto cleanup;
>  
> +if (testQemuAddRISCV32Guest(caps))
> +goto cleanup;
> +
> +if (testQemuAddRISCV64Guest(caps))
> +goto cleanup;

Despite the surrounding code suggesting otherwise, both these calls
should look like

  if (testQemuAddRISCV32Guest(caps) < 0)
goto cleanup;

Again, I'll take care of fixing the existing instances.


Once all of the above have been taken care of, the patch will look
good; however, I think it doesn't make a lot of sense to merge it
on its own, and we should rather squash it into 07/11 instead and
use a commit message like

  tests: Add RISC-V architectures

for the whole thing.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[libvirt] [PATCH] qemuDomainSaveMemory: Don't enforce dynamicOwnership

[Michal Privoznik]

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

When doing a memory snapshot qemuOpenFile() is used. This means
that the file where memory is saved is firstly attempted to be
created under root:root (because that's what libvirtd is running
under) and if this fails the second attempt is done under
domain's uid:gid. This does not make much sense - qemu is given
opened FD so it does not need to access the file. Moreover, if
dynamicOwnership is set in qemu.conf and the file lives on a
squashed NFS this is deadly combination and very likely to fail.

The fix consists of using:

  qemuOpenFileAs(fallback_uid = cfg->user,
 fallback_gid = cfg->group,
 dynamicOwnership = false)

In other words, dynamicOwnership is turned off for memory
snapshot (chown() will still be attempted if the file does not
live on NFS) and instead of using domain DAC label, configured
user:group is set as fallback.

Signed-off-by: Michal Privoznik 
---
 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 129bacdd34..6af7e6e171 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3223,6 +3223,7 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver,
  unsigned int flags,
  qemuDomainAsyncJob asyncJob)
 {
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 bool bypassSecurityDriver = false;
 bool needUnlink = false;
 int ret = -1;
@@ -3241,9 +3242,10 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver,
 goto cleanup;
 }
 }
-fd = qemuOpenFile(driver, vm, path,
-  O_WRONLY | O_TRUNC | O_CREAT | directFlag,
-  , );
+
+fd = qemuOpenFileAs(cfg->user, cfg->group, false, path,
+O_WRONLY | O_TRUNC | O_CREAT | directFlag,
+, );
 if (fd < 0)
 goto cleanup;
 
@@ -3283,6 +3285,7 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver,
  cleanup:
 VIR_FORCE_CLOSE(fd);
 virFileWrapperFdFree(wrapperFd);
+virObjectUnref(cfg);
 
 if (ret < 0 && needUnlink)
 unlink(path);
-- 
2.16.4

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

Re: [libvirt] [PATCH 2/2] qemu: Escape commas for qemuBuildSCSIiSCSIHostdevDrvStr

[Anya Harter]

On 06/26/2018 08:52 AM, John Ferlan wrote:
> 
> 
> On 06/22/2018 09:06 AM, Anya Harter wrote:
> [...]
> 
>>>
>>> Something is not quite right about seeing "example,foo.org" as a host
>>> name. I don't see that as a valid host name regardless of comma
>>> escaping. Do we escape commas in other "" string
>>> fields? I don't see it being done for qemuBuildChrChardevStr (search on
>>> TCP or "host.*name" in qemu_command.c)>
>>> Now the one thing that perhaps *could* be escaped is the  name
>>> attribute. Currently it's set to "iqn.1992-01.com.example", but the spec
>>> defines a structure of that name field to have a field that is a "String
>>> defined by the naming authority" (shorter version, see
>>> https://en.wikipedia.org/wiki/ISCSI)
>>>
>>> So if the name is : 'iqn.1992-01.com.example:storage/2', then the
>>> "storage/2" piece is the string defined by the naming authority. Not
>>> that I know whether it would work, but that's where I suppose a comma
>>> could go.
>>
>> If you could give me a string with a comma in it, I would be happy to
>> put it into the test instead of in the host name field. However, if no
>> one can think of a realistic case where comma escaping would be
>> necessary, we can just remove the test altogether. I only put it in as a
>> test that implementation was correct and that the commas were actually
>> being escaped correctly.
>>
> 
> Sorry for the delay - I stuck my head into the checkpoint/backup
> quicksand and it's been "difficult" to remove it ;-)
> 
> So going back to your analysis from the previous series:
> 
> https://www.redhat.com/archives/libvir-list/2018-June/msg01467.html
> 
> Now I see where/why you got here. The myopia I have is that host->name
> shouldn't have a comma in it as it's an invalid address format.
> 
> So, rather than:
> 
>   
> 
> if I instead use:
> 
>   
> 
> then with a couple of adjustments to input:
> 
> -
> +
> 
> ...
> 
> -  
> -
> +
> +
> 
> qemuxml2argvtest.c:
> 
> -QEMU_CAPS_SCSI_LSI,
> 
> and output:
> 
> --device lsi,id=scsi0,bus=pci.0,addr=0x3 \
> +-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 \
> 
> ...
> 
> --drive file=iscsi://example,,foo.org:3260/iqn.1992-01.com.example/0,if=none,\
> -format=raw,id=drive-hostdev0 \
> --device scsi-generic,bus=scsi0.0,scsi-id=4,drive=drive-hostdev0,id=hostdev0 \
> +-drive file=iscsi://example.foo.org:3260/iqn.1992-01.com.example%3Amy,,\
> +storage/1,if=none,format=raw,id=drive-hostdev0 \
> +-device scsi-generic,bus=scsi0.0,channel=0,scsi-id=0,lun=4,\
> +drive=drive-hostdev0,id=hostdev0 \
> 
> Then, I think we cover this oddball case.
> 
> I can make those changes for you before pushing as long as you
> agree that's fine. In the long run, it's similar, it's just avoiding
> the invalid hostname/uri string.

I think that sounds good.

Thanks,

Anya

> 
> John
> 
> and yes, once this is pushed, that item comes off the wiki list.
> 
> [...]
> 

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

Re: [libvirt] [PATCH v2 07/11] tests/capabilitiestest: add RISC-V capability tests

[Andrea Bolognani]

On Thu, 2018-06-14 at 22:32 +0200, Lubomir Rintel wrote:
[...]
> +{
> +  "execute": "query-version",
> +  "id": "libvirt-2"
> +}
> +
> +{
> +  "return": {
> +"qemu": {
> +  "micro": 93,
> +  "minor": 11,
> +  "major": 2
> +},
> +"package": "qemu-2.12.0-0.7.rc3.fc29"
> +  },
> +  "id": "libvirt-2"
> +}

Replies should ideally be generated against official releases built
from upstream sources. I know you could easily point out instances
of that *not* being the case in the existing data, so it's not a
blocker per-se - just pointing out how it *should* be done :)

That said, I've recently pushed some changes that unfortunately
require the data to be regenerated again, so perhaps you could take
the opportunity to pull it from a released QEMU binary? Ideally one
that you built yourself from vanilla upstream, but even a Fedora
package for 2.12.0 proper would be preferable to the above.

Everything else looks good.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH v2] qemuDomainObjBeginJobInternal: Report agent job in error message

[John Ferlan]

On 06/25/2018 02:01 AM, Michal Privoznik wrote:
> If a thread is unable to acquire a job (e.g. because of timeout)
> an error is reported and the error message contains reference to
> the other thread holding the job. Well, the error message should
> report agent job too as it is yet another source of possible
> failure.
> 
> Signed-off-by: Michal Privoznik 
> ---
> 
> diff to v1:
> - Expand messages to cover all @blocker and @agentBlocker possibilities
>   (as Jira requested in review)
> 
>  src/qemu/qemu_domain.c | 46 ++
>  1 file changed, 38 insertions(+), 8 deletions(-)
> 

Reviewed-by: John Ferlan 

John

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

Re: [libvirt] [PATCH v2 04/11] docs/schemas: add RISC-V architectures

[Andrea Bolognani]

On Thu, 2018-06-14 at 22:32 +0200, Lubomir Rintel wrote:
> Signed-off-by: Lubomir Rintel 
> ---
>  docs/schemas/basictypes.rng | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
> index 1a18cd31b1..14a3670e5c 100644
> --- a/docs/schemas/basictypes.rng
> +++ b/docs/schemas/basictypes.rng
> @@ -398,6 +398,8 @@
>ppc64
>ppc64le
>ppcemb
> +  riscv32
> +  riscv64
>s390
>s390x
>sh4

This can be squashed in with the previous patch.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH v2 03/11] util: add RISC-V architectures

[Andrea Bolognani]

On Thu, 2018-06-14 at 22:32 +0200, Lubomir Rintel wrote:
[...]
> diff --git a/src/util/virarch.c b/src/util/virarch.c
> index be48bcfb89..77b893b0ac 100644
> --- a/src/util/virarch.c
> +++ b/src/util/virarch.c
> @@ -65,6 +65,9 @@ static const struct virArchData {
>  { "ppc64le",  64, VIR_ARCH_LITTLE_ENDIAN },
>  { "ppcemb",   32, VIR_ARCH_BIG_ENDIAN },
>  
> +{ "riscv32",  32, VIR_ARCH_LITTLE_ENDIAN },
> +{ "riscv64",  64, VIR_ARCH_LITTLE_ENDIAN },
> +
>  { "s390", 32, VIR_ARCH_BIG_ENDIAN },
>  { "s390x",64, VIR_ARCH_BIG_ENDIAN },
>  { "sh4",  32, VIR_ARCH_LITTLE_ENDIAN },
> diff --git a/src/util/virarch.h b/src/util/virarch.h
> index af5ff83528..806a23fade 100644
> --- a/src/util/virarch.h
> +++ b/src/util/virarch.h
> @@ -55,6 +55,9 @@ typedef enum {
>  VIR_ARCH_PPC64LE,  /* PowerPC 64 LE 
> http://en.wikipedia.org/wiki/PowerPC */
>  VIR_ARCH_PPCEMB,   /* PowerPC 32 BE 
> http://en.wikipedia.org/wiki/PowerPC */
>  
> +VIR_ARCH_RISCV32,  /* RISC-V 64 LE 
> http://en.wikipedia.org/wiki/RISC-V */
> +VIR_ARCH_RISCV64,  /* RISC-V 32 BE 
> http://en.wikipedia.org/wiki/RISC-V */
> +
>  VIR_ARCH_S390, /* S39032 BE 
> http://en.wikipedia.org/wiki/S390 */
>  VIR_ARCH_S390X,/* S39064 BE 
> http://en.wikipedia.org/wiki/S390x */
>  VIR_ARCH_SH4,  /* SuperH4 32 LE 
> http://en.wikipedia.org/wiki/SuperH */

Both here and in the header file, we keep enum entries grouped
five by five, so you'll have to do a little regrouping...

With that taken care of

  Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH 2/2] qemu: Escape commas for qemuBuildSCSIiSCSIHostdevDrvStr

[John Ferlan]

On 06/22/2018 09:06 AM, Anya Harter wrote:
[...]

>>
>> Something is not quite right about seeing "example,foo.org" as a host
>> name. I don't see that as a valid host name regardless of comma
>> escaping. Do we escape commas in other "" string
>> fields? I don't see it being done for qemuBuildChrChardevStr (search on
>> TCP or "host.*name" in qemu_command.c)>
>> Now the one thing that perhaps *could* be escaped is the  name
>> attribute. Currently it's set to "iqn.1992-01.com.example", but the spec
>> defines a structure of that name field to have a field that is a "String
>> defined by the naming authority" (shorter version, see
>> https://en.wikipedia.org/wiki/ISCSI)
>>
>> So if the name is : 'iqn.1992-01.com.example:storage/2', then the
>> "storage/2" piece is the string defined by the naming authority. Not
>> that I know whether it would work, but that's where I suppose a comma
>> could go.
> 
> If you could give me a string with a comma in it, I would be happy to
> put it into the test instead of in the host name field. However, if no
> one can think of a realistic case where comma escaping would be
> necessary, we can just remove the test altogether. I only put it in as a
> test that implementation was correct and that the commas were actually
> being escaped correctly.
> 

Sorry for the delay - I stuck my head into the checkpoint/backup
quicksand and it's been "difficult" to remove it ;-)

So going back to your analysis from the previous series:

https://www.redhat.com/archives/libvir-list/2018-June/msg01467.html

Now I see where/why you got here. The myopia I have is that host->name
shouldn't have a comma in it as it's an invalid address format.

So, rather than:

  

if I instead use:

  

then with a couple of adjustments to input:

-
+

...

-  
-
+
+

qemuxml2argvtest.c:

-QEMU_CAPS_SCSI_LSI,

and output:

--device lsi,id=scsi0,bus=pci.0,addr=0x3 \
+-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 \

...

--drive file=iscsi://example,,foo.org:3260/iqn.1992-01.com.example/0,if=none,\
-format=raw,id=drive-hostdev0 \
--device scsi-generic,bus=scsi0.0,scsi-id=4,drive=drive-hostdev0,id=hostdev0 \
+-drive file=iscsi://example.foo.org:3260/iqn.1992-01.com.example%3Amy,,\
+storage/1,if=none,format=raw,id=drive-hostdev0 \
+-device scsi-generic,bus=scsi0.0,channel=0,scsi-id=0,lun=4,\
+drive=drive-hostdev0,id=hostdev0 \

Then, I think we cover this oddball case.

I can make those changes for you before pushing as long as you
agree that's fine. In the long run, it's similar, it's just avoiding
the invalid hostname/uri string.

John

and yes, once this is pushed, that item comes off the wiki list.

[...]

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

Re: [libvirt] [PATCH v2 02/11] qemu: rename qemuDomainArmVirt()

[Andrea Bolognani]

On Thu, 2018-06-14 at 22:32 +0200, Lubomir Rintel wrote:
> It's ARM specific.
> 
> Signed-off-by: Lubomir Rintel 
> ---
>  src/qemu/qemu_capabilities.c   |  4 ++--
>  src/qemu/qemu_command.c|  4 ++--
>  src/qemu/qemu_domain.c | 16 
>  src/qemu/qemu_domain.h |  2 +-
>  src/qemu/qemu_domain_address.c |  4 ++--
>  5 files changed, 15 insertions(+), 15 deletions(-)

With this patch squashed into the previous one, and the subject
updated accordingly to read something like

  qemu: Rename qemuDomain*IsVirt() to qemuDomain*IsARMVirt()

you have my

  Reviewed-by: Andrea Bolognani 

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH 1/8] snapshots: Avoid term 'checkpoint' for full system snapshot

[John Ferlan]

On 06/25/2018 06:27 PM, Eric Blake wrote:
> On 06/22/2018 04:16 PM, John Ferlan wrote:
>>
>>
>> On 06/13/2018 12:42 PM, Eric Blake wrote:
>>> Upcoming patches plan to introduce virDomainCheckpointPtr as a new
>>> object for use in incremental backups, along with documentation
>>> how incremental backups differ from snapshots.  But first, we need
>>> to rename any existing mention of a 'system checkpoint' to instead
>>> be a 'full system state snapshot', so that we aren't overloading
>>> the term checkpoint.
>>>
>>> Signed-off-by: Eric Blake 
>>>
>>> ---
>>> Bikeshed suggestions on what to name the new object for use in
>>> backups is welcome, if we would rather keep the term 'checkpoint'
>>> for a disk+memory snapshot.
>>> ---
>>
>> "Naming is hard" and opinions can vary greatly - be careful for what you
>> ask in case you receive something not wanted ;-).
>>
>> I haven't followed the discussions thus far all that closely, but I'll
>> give this a go anyway since it's languishing and saying nothing is akin
>> to implicitly agreeing everything is fine. Fair warning, I'm not all
>> that familiar with snapshot algorithms having largely tried to ignore it
>> since others (Eric and Peter) have far more in depth knowledge.
>>
>> In any case, another option for the proposed "checkpoint" could be a
>> "snapshot reference". One can start or end a reference period and then
>> set or clear a reference point.
>>
>> What I'm not clear on yet is whether the intention is to have this
>> checkpoint (and backup) be integrated in any way to the existing
>> snapshot algorithms. I guess part of me thinks that if I take a full
>> system snapshot, then any backup/checkpoint data should be included so
>> that if/when I go back to that point in time I can start from whence I
>> left as it relates to my backup. Kind of a superset and/or integrated
>> model rather than something bolted onto the side to resolve a specific
>> need.
> 
> That's a tough call.  My current design has incremental backups
> completely separate from the existing checkpoint code for several reasons:
> - the snapshot code is already confusing with lots of flags
> (internal/external, disk/memory, etc)
> - snapshots can be reverted to (well, in theory - we STILL can't revert
> to an external snapshot cleanly, even though the design supports it)
> - incremental backups are not direct revert points
> 
> so, rather than bolt something on to the existing design, I went with a
> new concept. As you found later in the series, I then tried to provide a
> good summary page describing the different pieces, and what tradeoffs
> are involved in order to know which approach will work for a given need.
> 

Understood - since I've made it further now. Domain state is somewhat of
a tangled or interwoven part of the domain XML lifecycle fabric, so I
perhaps process it that way when reading new code.

It seems the design is that XML for domain{checkpoint|backup} will be
similar to domainstatus, but far more restrictive to the specific needs
for each since you're saving the original config in the output.

>>
>> I suppose a reservation I have about separate virDomainCheckpoint* and
>> virDomainBackup* API's is understanding the relationship between the two
>> naming spaces. IIUC though a Checkpoint would be reference point in time
>> within a Backup period.
> 
> A sequence of snapshots are different points in time you can revert to.
> A sequence of checkpoints are different points in time you can use as
> the reference point for starting an incremental backup.
> 
> So if we don't like the term 'checkpoint', maybe
> virDomainBlockBackupReference would work.  But it is longer, and would
> make for some mouthful API names.

I saw things as more of "{Create|Start|Set|Clear|End|Destroy}" type
operations initially, but I've gone through my viewpoint has changed.
I'm still concerned with over complicating with too many flags which is
where we seem to have gotten ourselves in trouble with snapshot as time
went on and more or less functionality points were desired.

Checkpoint/backup is complex enough without adding levels of features
for consumers that may or may not be used. In the long run, it's where
do you give up control and how much can/should be assumed to be
libvirt's "job".

If a consumer wants to do something particularly tricky, then maybe we
should just hand them the gun with one bullet in the chamber (so to
speak). Provide an API that "locks out" other threads instead of doing
that for them ;-) - someone always thinks they can do it better!

In the long run, we don't necessarily know how all consumers would like
to use this and so far there's been mixed opinions on what should be
done. At some level the operation of starting checkpoints, setting
checkpoints, and allowing/performing backups from a specific checkpoint
are fairly straightforward type operations. It's the additional cruft
and flags to do "fancy" stuff or the desire to worry about every
possible operation 

Re: [libvirt] [PATCH v2 0/2] Alter qemu live/cold device attach algorithm

[Michal Privoznik]

On 06/26/2018 12:36 PM, John Ferlan wrote:
> 
> ping^2?
> 
> No other opinions?
> 
> Tks,
> 
> John
> 
> On 06/20/2018 07:14 PM, John Ferlan wrote:
>>
>>
>> On 06/12/2018 10:32 AM, John Ferlan wrote:
>>> v1: https://www.redhat.com/archives/libvir-list/2018-June/msg00465.html
>>>
>>> The first patch is essentially a repeat of the v1 patch, but with
>>> a more correct commit message. During testing if the same address
>>> was supplied for the --config it wasn't checked.
>>>
>>> The second patch rearranges qemuDomainAttachDeviceLiveAndConfig to
>>> perform CONFIG and LIVE actions separately since the generated
>>> @devConf or @devLive could be different depending on whether the
>>> incoming XML has an  or not and whether the corresponding
>>> domain parse and post parse algorithms need to generate some sort
>>> of address for the device.
>>>
>>>
>>> John Ferlan (2):
>>>   qemu: Check for existing hostdev address for cold attach device
>>>   qemu: Use the correct vm def on cold attach
>>>
>>>  src/qemu/qemu_driver.c | 58 
>>> +-
>>>  1 file changed, 29 insertions(+), 29 deletions(-)
>>>
>>
>> ping?
>>
>> Can we come to a consensus either way on this? There's 2 that feel
>> fixing the underlying problem is worth it (myself and Laine) and 1 that
>> would not bother fixing the bug (Jano).

I think it's worth fixing too.

Michal

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

Re: [libvirt] [PATCH] set-lifecycle-action: add description of type and action

[Michal Privoznik]

On 06/21/2018 01:28 PM, Chen Hanxiao wrote:
> From: Chen Hanxiao 
> 
> In [1],  are described as "on_poweroff",
>"on_reboot", "on_crash".
>but we accept "poweroff", "reboot" and "crash".
> This patch adds docs about them.
> 
> [1]: https://libvirt.org/formatdomain.html#elementsEvents
> 
> Signed-off-by: Chen Hanxiao 
> ---
>  tools/virsh.pod | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 7cb8c8a6e4..1b479f318d 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -2414,8 +2414,12 @@ the B command.
>  =item B I I I
>  [[I<--config>] [I<--live>] | [I<--current>]]
>  
> -Set the lifecycle I for specified lifecycle I. For the list of
> -lifecycle types and actions and possible combinations see the documentation 
> at
> +Set the lifecycle I for specified lifecycle I.
> +The valid I are "poweroff", "reboot" and "crash", each of them
> +allow valid I are "destroy", "restart", "rename-restart", "preserve".
> +For I "crash", additional actions "coredump-destroy"
> +and "coredump-restart" are supported. For the list of lifecycle types
> +and actions and possible combinations see the documentation at
>  L.

I've reworded this block slightly and removed reference to the web page,
because we are enumerating types and actions here.

ACKed and pushed.

Michal

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

Re: [libvirt] [PATCH v3 00/10] Storage encryption adjustments

[Michal Privoznik]

On 06/21/2018 01:01 AM, John Ferlan wrote:
> v2: https://www.redhat.com/archives/libvir-list/2018-May/msg01268.html
> 
> Try #3 - this time since Peter removed support for qcow encrypted
> volumes for domains, I'm taking the same approach for storage.
> 
> This is totally different from the previous approach which tried to
> actually create a qcow2 encrypted volume. This time slowly extricate
> the qcow2 encryption support from the storage driver - at least anything
> that can have a result via qemu-img.
> 
> Additionally, added some more luks tests and added the capability
> to create a luks encrypted volume from a raw image using the two
> step process that was part of v2.
> 
> John Ferlan (10):
>   storage: Don't allow encryption secretPath to be NULL
>   tests: Add luks creation examples to storagevolxml2argvtest
>   storage: Rename encryption info variable for clarity
>   tests: Remove qcow2 encryption from storagevol tests
>   storage: Disallow create/resize of qcow2 encrypted images
>   storage: Clean up storageBackendCreateQemuImgCheckEncryption
>   storage: Clean up storageBackendCreateQemuImgOpts
>   storage: Remove storageBackendGenerateSecretData
>   storage: Add support for using inputvol for encryption
>   docs: Add news article for volume encryption modifications
> 
>  docs/formatsecret.html.in  |  22 +-
>  docs/formatstorageencryption.html.in   |  29 +-
>  docs/news.xml  |  25 ++
>  src/storage/storage_util.c | 303 
> +++--
>  src/storage/storage_util.h |  10 +-
>  src/util/virqemu.c |  26 +-
>  tests/storagevolxml2argvdata/luks-cipher.argv  |   5 +
>  tests/storagevolxml2argvdata/luks-convert.argv |   9 +
>  tests/storagevolxml2argvdata/luks.argv |   4 +
>  tests/storagevolxml2argvdata/qcow2-1.1.argv|   2 +-
>  tests/storagevolxml2argvdata/qcow2-compat.argv |   2 +-
>  .../qcow2-from-logical-compat.argv |   2 +-
>  tests/storagevolxml2argvdata/qcow2-lazy.argv   |   2 +-
>  .../qcow2-nobacking-convert-prealloc-compat.argv   |   2 +-
>  .../qcow2-nobacking-prealloc-compat.argv   |   2 +-
>  .../qcow2-nocapacity-convert-prealloc.argv |   2 +-
>  tests/storagevolxml2argvdata/qcow2-nocapacity.argv |   2 +-
>  .../storagevolxml2argvdata/qcow2-nocow-compat.argv |   2 +-
>  tests/storagevolxml2argvtest.c |  76 +-
>  tests/storagevolxml2xmlin/vol-luks-convert.xml |  21 ++
>  tests/storagevolxml2xmlin/vol-qcow2-0.10-lazy.xml  |   3 -
>  tests/storagevolxml2xmlin/vol-qcow2-1.1.xml|   3 -
>  tests/storagevolxml2xmlin/vol-qcow2-encryption.xml |  31 +++
>  tests/storagevolxml2xmlin/vol-qcow2-lazy.xml   |   3 -
>  tests/storagevolxml2xmlin/vol-qcow2-nobacking.xml  |   3 -
>  .../vol-qcow2-nocapacity-backing.xml   |   3 -
>  tests/storagevolxml2xmlin/vol-qcow2-nocapacity.xml |   3 -
>  tests/storagevolxml2xmlin/vol-qcow2-nocow.xml  |   3 -
>  tests/storagevolxml2xmlin/vol-qcow2.xml|   3 -
>  tests/storagevolxml2xmlout/vol-qcow2-0.10-lazy.xml |   3 -
>  tests/storagevolxml2xmlout/vol-qcow2-1.1.xml   |   3 -
>  .../storagevolxml2xmlout/vol-qcow2-encryption.xml  |  31 +++
>  tests/storagevolxml2xmlout/vol-qcow2-lazy.xml  |   3 -
>  tests/storagevolxml2xmlout/vol-qcow2-nobacking.xml |   3 -
>  .../storagevolxml2xmlout/vol-qcow2-nocapacity.xml  |   3 -
>  tests/storagevolxml2xmlout/vol-qcow2-nocow.xml |   3 -
>  tests/storagevolxml2xmlout/vol-qcow2.xml   |   3 -
>  tests/storagevolxml2xmltest.c  |   1 +
>  38 files changed, 344 insertions(+), 312 deletions(-)
>  create mode 100644 tests/storagevolxml2argvdata/luks-cipher.argv
>  create mode 100644 tests/storagevolxml2argvdata/luks-convert.argv
>  create mode 100644 tests/storagevolxml2argvdata/luks.argv
>  create mode 100644 tests/storagevolxml2xmlin/vol-luks-convert.xml
>  create mode 100644 tests/storagevolxml2xmlin/vol-qcow2-encryption.xml
>  create mode 100644 tests/storagevolxml2xmlout/vol-qcow2-encryption.xml
> 

ACK series.

Michal

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

[libvirt] [PATCH] qemu: hotplug: fix mdev attach for vfio-ccw

[Bjoern Walk]

Mediated devices of model 'vfio-ccw' are using CCW addresses, so make
sure to call the correct address preparation code for the model.

Reviewed-by: Shalini Chellathurai Saroja 
Reviewed-by: Boris Fiuczynski 
Signed-off-by: Bjoern Walk 
---
 src/qemu/qemu_hotplug.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 7a1bbc7c..46d30ddd 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2446,8 +2446,15 @@ qemuDomainAttachMediatedDevice(virQEMUDriverPtr driver,
 virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_HOSTDEV,
 { .hostdev = hostdev } };
 
-if (qemuDomainEnsurePCIAddress(vm, , driver) < 0)
-return -1;
+switch (hostdev->source.subsys.u.mdev.model) {
+case VIR_MDEV_MODEL_TYPE_VFIO_PCI:
+if (qemuDomainEnsurePCIAddress(vm, , driver) < 0)
+return -1;
+break;
+case VIR_MDEV_MODEL_TYPE_VFIO_CCW:
+case VIR_MDEV_MODEL_TYPE_LAST:
+break;
+}
 
 if (qemuHostdevPrepareMediatedDevices(driver,
   vm->def->name,
-- 
2.17.0

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

[libvirt] [PATCH] virsh: Provide completer for detach-device-alias

[Michal Privoznik]

Signed-off-by: Michal Privoznik 
---
 tools/virsh-completer.c | 48 
 tools/virsh-completer.h |  4 
 tools/virsh-domain.c|  1 +
 3 files changed, 53 insertions(+)

diff --git a/tools/virsh-completer.c b/tools/virsh-completer.c
index d3effe59ea..ae579a5bdb 100644
--- a/tools/virsh-completer.c
+++ b/tools/virsh-completer.c
@@ -811,3 +811,51 @@ virshCellnoCompleter(vshControl *ctl,
 VIR_FREE(ret);
 goto cleanup;
 }
+
+
+char **
+virshDomainDeviceAliasCompleter(vshControl *ctl,
+const vshCmd *cmd,
+unsigned int flags)
+{
+virshControlPtr priv = ctl->privData;
+xmlDocPtr xmldoc = NULL;
+xmlXPathContextPtr ctxt = NULL;
+int naliases;
+xmlNodePtr *aliases = NULL;
+size_t i;
+unsigned int domainXMLFlags = 0;
+char **ret = NULL;
+char **tmp = NULL;
+
+virCheckFlags(0, NULL);
+
+if (!priv->conn || virConnectIsAlive(priv->conn) <= 0)
+return NULL;
+
+if (vshCommandOptBool(cmd, "config"))
+domainXMLFlags = VIR_DOMAIN_XML_INACTIVE;
+
+if (virshDomainGetXML(ctl, cmd, domainXMLFlags, , ) < 0)
+goto cleanup;
+
+naliases = virXPathNodeSet("./devices//alias/@name", ctxt, );
+if (naliases < 0)
+goto cleanup;
+
+if (VIR_ALLOC_N(tmp, naliases + 1) < 0)
+goto cleanup;
+
+for (i = 0; i < naliases; i++) {
+if (!(tmp[i] = virXMLNodeContentString(aliases[i])))
+goto cleanup;
+}
+
+VIR_STEAL_PTR(ret, tmp);
+ cleanup:
+VIR_FREE(aliases);
+xmlFreeDoc(xmldoc);
+xmlXPathFreeContext(ctxt);
+virStringListFree(tmp);
+return ret;
+}
diff --git a/tools/virsh-completer.h b/tools/virsh-completer.h
index ee7eec68c5..50dc068d73 100644
--- a/tools/virsh-completer.h
+++ b/tools/virsh-completer.h
@@ -94,6 +94,10 @@ char ** virshNodedevEventNameCompleter(vshControl *ctl,
const vshCmd *cmd,
unsigned int flags);
 
+char ** virshDomainDeviceAliasCompleter(vshControl *ctl,
+const vshCmd *cmd,
+unsigned int flags);
+
 char ** virshCellnoCompleter(vshControl *ctl,
  const vshCmd *cmd,
  unsigned int flags);
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 6aa79f11b9..e9b88f0013 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11684,6 +11684,7 @@ static const vshCmdOptDef opts_detach_device_alias[] = {
 {.name = "alias",
  .type = VSH_OT_DATA,
  .flags = VSH_OFLAG_REQ,
+ .completer = virshDomainDeviceAliasCompleter,
  .help = N_("device alias")
 },
 VIRSH_COMMON_OPT_DOMAIN_CONFIG,
-- 
2.16.4

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

Re: [libvirt] [PATCH v2 0/2] Alter qemu live/cold device attach algorithm

[John Ferlan]

ping^2?

No other opinions?

Tks,

John

On 06/20/2018 07:14 PM, John Ferlan wrote:
> 
> 
> On 06/12/2018 10:32 AM, John Ferlan wrote:
>> v1: https://www.redhat.com/archives/libvir-list/2018-June/msg00465.html
>>
>> The first patch is essentially a repeat of the v1 patch, but with
>> a more correct commit message. During testing if the same address
>> was supplied for the --config it wasn't checked.
>>
>> The second patch rearranges qemuDomainAttachDeviceLiveAndConfig to
>> perform CONFIG and LIVE actions separately since the generated
>> @devConf or @devLive could be different depending on whether the
>> incoming XML has an  or not and whether the corresponding
>> domain parse and post parse algorithms need to generate some sort
>> of address for the device.
>>
>>
>> John Ferlan (2):
>>   qemu: Check for existing hostdev address for cold attach device
>>   qemu: Use the correct vm def on cold attach
>>
>>  src/qemu/qemu_driver.c | 58 
>> +-
>>  1 file changed, 29 insertions(+), 29 deletions(-)
>>
> 
> ping?
> 
> Can we come to a consensus either way on this? There's 2 that feel
> fixing the underlying problem is worth it (myself and Laine) and 1 that
> would not bother fixing the bug (Jano).
> 
> Tks -
> 
> John
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 

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

Re: [libvirt] [PATCH v3 00/10] Storage encryption adjustments

[John Ferlan]

ping?

Tks,

John

On 06/20/2018 07:01 PM, John Ferlan wrote:
> v2: https://www.redhat.com/archives/libvir-list/2018-May/msg01268.html
> 
> Try #3 - this time since Peter removed support for qcow encrypted
> volumes for domains, I'm taking the same approach for storage.
> 
> This is totally different from the previous approach which tried to
> actually create a qcow2 encrypted volume. This time slowly extricate
> the qcow2 encryption support from the storage driver - at least anything
> that can have a result via qemu-img.
> 
> Additionally, added some more luks tests and added the capability
> to create a luks encrypted volume from a raw image using the two
> step process that was part of v2.
> 
> John Ferlan (10):
>   storage: Don't allow encryption secretPath to be NULL
>   tests: Add luks creation examples to storagevolxml2argvtest
>   storage: Rename encryption info variable for clarity
>   tests: Remove qcow2 encryption from storagevol tests
>   storage: Disallow create/resize of qcow2 encrypted images
>   storage: Clean up storageBackendCreateQemuImgCheckEncryption
>   storage: Clean up storageBackendCreateQemuImgOpts
>   storage: Remove storageBackendGenerateSecretData
>   storage: Add support for using inputvol for encryption
>   docs: Add news article for volume encryption modifications
> 
>  docs/formatsecret.html.in  |  22 +-
>  docs/formatstorageencryption.html.in   |  29 +-
>  docs/news.xml  |  25 ++
>  src/storage/storage_util.c | 303 
> +++--
>  src/storage/storage_util.h |  10 +-
>  src/util/virqemu.c |  26 +-
>  tests/storagevolxml2argvdata/luks-cipher.argv  |   5 +
>  tests/storagevolxml2argvdata/luks-convert.argv |   9 +
>  tests/storagevolxml2argvdata/luks.argv |   4 +
>  tests/storagevolxml2argvdata/qcow2-1.1.argv|   2 +-
>  tests/storagevolxml2argvdata/qcow2-compat.argv |   2 +-
>  .../qcow2-from-logical-compat.argv |   2 +-
>  tests/storagevolxml2argvdata/qcow2-lazy.argv   |   2 +-
>  .../qcow2-nobacking-convert-prealloc-compat.argv   |   2 +-
>  .../qcow2-nobacking-prealloc-compat.argv   |   2 +-
>  .../qcow2-nocapacity-convert-prealloc.argv |   2 +-
>  tests/storagevolxml2argvdata/qcow2-nocapacity.argv |   2 +-
>  .../storagevolxml2argvdata/qcow2-nocow-compat.argv |   2 +-
>  tests/storagevolxml2argvtest.c |  76 +-
>  tests/storagevolxml2xmlin/vol-luks-convert.xml |  21 ++
>  tests/storagevolxml2xmlin/vol-qcow2-0.10-lazy.xml  |   3 -
>  tests/storagevolxml2xmlin/vol-qcow2-1.1.xml|   3 -
>  tests/storagevolxml2xmlin/vol-qcow2-encryption.xml |  31 +++
>  tests/storagevolxml2xmlin/vol-qcow2-lazy.xml   |   3 -
>  tests/storagevolxml2xmlin/vol-qcow2-nobacking.xml  |   3 -
>  .../vol-qcow2-nocapacity-backing.xml   |   3 -
>  tests/storagevolxml2xmlin/vol-qcow2-nocapacity.xml |   3 -
>  tests/storagevolxml2xmlin/vol-qcow2-nocow.xml  |   3 -
>  tests/storagevolxml2xmlin/vol-qcow2.xml|   3 -
>  tests/storagevolxml2xmlout/vol-qcow2-0.10-lazy.xml |   3 -
>  tests/storagevolxml2xmlout/vol-qcow2-1.1.xml   |   3 -
>  .../storagevolxml2xmlout/vol-qcow2-encryption.xml  |  31 +++
>  tests/storagevolxml2xmlout/vol-qcow2-lazy.xml  |   3 -
>  tests/storagevolxml2xmlout/vol-qcow2-nobacking.xml |   3 -
>  .../storagevolxml2xmlout/vol-qcow2-nocapacity.xml  |   3 -
>  tests/storagevolxml2xmlout/vol-qcow2-nocow.xml |   3 -
>  tests/storagevolxml2xmlout/vol-qcow2.xml   |   3 -
>  tests/storagevolxml2xmltest.c  |   1 +
>  38 files changed, 344 insertions(+), 312 deletions(-)
>  create mode 100644 tests/storagevolxml2argvdata/luks-cipher.argv
>  create mode 100644 tests/storagevolxml2argvdata/luks-convert.argv
>  create mode 100644 tests/storagevolxml2argvdata/luks.argv
>  create mode 100644 tests/storagevolxml2xmlin/vol-luks-convert.xml
>  create mode 100644 tests/storagevolxml2xmlin/vol-qcow2-encryption.xml
>  create mode 100644 tests/storagevolxml2xmlout/vol-qcow2-encryption.xml
> 

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

[libvirt] [PATCH v4 2/6] nwfilter: keep track of active filter bindings

[Daniel P . Berrangé]

Currently the nwfilter driver does not keep any record of what filter
bindings it has active. This means that when it needs to recreate
filters, it has to rely on triggering callbacks provided by the virt
drivers. This introduces a hash table recording the virNWFilterBinding
objects so the driver has a record of all active filters.

Signed-off-by: Daniel P. Berrangé 
---
 src/conf/virnwfilterobj.h  |  4 ++
 src/nwfilter/nwfilter_driver.c | 84 --
 2 files changed, 63 insertions(+), 25 deletions(-)

diff --git a/src/conf/virnwfilterobj.h b/src/conf/virnwfilterobj.h
index 433b0402d0..4a54dd50da 100644
--- a/src/conf/virnwfilterobj.h
+++ b/src/conf/virnwfilterobj.h
@@ -22,6 +22,7 @@
 # include "internal.h"
 
 # include "nwfilter_conf.h"
+# include "virnwfilterbindingobjlist.h"
 
 typedef struct _virNWFilterObj virNWFilterObj;
 typedef virNWFilterObj *virNWFilterObjPtr;
@@ -37,7 +38,10 @@ struct _virNWFilterDriverState {
 
 virNWFilterObjListPtr nwfilters;
 
+virNWFilterBindingObjListPtr bindings;
+
 char *configDir;
+char *bindingDir;
 };
 
 virNWFilterDefPtr
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index 7202691646..1449b67c72 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -38,7 +38,6 @@
 #include "domain_conf.h"
 #include "domain_nwfilter.h"
 #include "nwfilter_driver.h"
-#include "virnwfilterbindingdef.h"
 #include "nwfilter_gentech_driver.h"
 #include "configmake.h"
 #include "virfile.h"
@@ -174,7 +173,6 @@ nwfilterStateInitialize(bool privileged,
 virStateInhibitCallback callback ATTRIBUTE_UNUSED,
 void *opaque ATTRIBUTE_UNUSED)
 {
-char *base = NULL;
 DBusConnection *sysbus = NULL;
 
 if (virDBusHasSystemBus() &&
@@ -191,6 +189,9 @@ nwfilterStateInitialize(bool privileged,
 if (!(driver->nwfilters = virNWFilterObjListNew()))
 goto error;
 
+if (!(driver->bindings = virNWFilterBindingObjListNew()))
+goto error;
+
 if (!privileged)
 return 0;
 
@@ -230,30 +231,35 @@ nwfilterStateInitialize(bool privileged,
 goto error;
 }
 
-if (VIR_STRDUP(base, SYSCONFDIR "/libvirt") < 0)
+if (VIR_STRDUP(driver->configDir, SYSCONFDIR "/libvirt/nwfilter") < 0)
 goto error;
 
-if (virAsprintf(>configDir,
-"%s/nwfilter", base) == -1)
+if (virFileMakePathWithMode(driver->configDir, S_IRWXU) < 0) {
+virReportSystemError(errno, _("cannot create config directory '%s'"),
+ driver->configDir);
 goto error;
+}
 
-VIR_FREE(base);
+if (VIR_STRDUP(driver->bindingDir, LOCALSTATEDIR 
"/run/libvirt/nwfilter-binding") < 0)
+goto error;
 
-if (virFileMakePathWithMode(driver->configDir, S_IRWXU) < 0) {
+if (virFileMakePathWithMode(driver->bindingDir, S_IRWXU) < 0) {
 virReportSystemError(errno, _("cannot create config directory '%s'"),
- driver->configDir);
+ driver->bindingDir);
 goto error;
 }
 
 if (virNWFilterObjListLoadAllConfigs(driver->nwfilters, driver->configDir) 
< 0)
 goto error;
 
+if (virNWFilterBindingObjListLoadAllConfigs(driver->bindings, 
driver->bindingDir) < 0)
+goto error;
+
 nwfilterDriverUnlock();
 
 return 0;
 
  error:
-VIR_FREE(base);
 nwfilterDriverUnlock();
 nwfilterStateCleanup();
 
@@ -333,9 +339,12 @@ nwfilterStateCleanup(void)
 nwfilterDriverRemoveDBusMatches();
 
 VIR_FREE(driver->configDir);
+VIR_FREE(driver->bindingDir);
 nwfilterDriverUnlock();
 }
 
+virObjectUnref(driver->bindings);
+
 /* free inactive nwfilters */
 virNWFilterObjListFree(driver->nwfilters);
 
@@ -647,13 +656,35 @@ nwfilterInstantiateFilter(const char *vmname,
   const unsigned char *vmuuid,
   virDomainNetDefPtr net)
 {
-virNWFilterBindingDefPtr binding;
+virNWFilterBindingObjPtr obj;
+virNWFilterBindingDefPtr def;
 int ret;
 
-if (!(binding = virNWFilterBindingDefForNet(vmname, vmuuid, net)))
+obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, 
net->ifname);
+if (obj) {
+virNWFilterBindingObjEndAPI();
+return 0;
+}
+
+if (!(def = virNWFilterBindingDefForNet(vmname, vmuuid, net)))
+return -1;
+
+obj = virNWFilterBindingObjListAdd(driver->bindings,
+   def);
+if (!obj) {
+virNWFilterBindingDefFree(def);
 return -1;
-ret = virNWFilterInstantiateFilter(driver, binding);
-virNWFilterBindingDefFree(binding);
+}
+
+ret = virNWFilterInstantiateFilter(driver, def);
+
+if (ret >= 0)
+virNWFilterBindingObjSave(obj, driver->bindingDir);
+else
+virNWFilterBindingObjListRemove(driver->bindings, obj);
+
+

[libvirt] [PATCH v4 3/6] nwfilter: remove virt driver callback layer for rebuilding filters

[Daniel P . Berrangé]

Now that the nwfilter driver keeps a list of bindings that it has
created, there is no need for the complex virt driver callbacks. It is
possible to simply iterate of the list of recorded filter bindings.

This means that rebuilding filters no longer has to acquire any locks on
the virDomainObj objects, as they're never touched.

Reviewed-by: John Ferlan 
Signed-off-by: Daniel P. Berrangé 
---
 src/conf/nwfilter_conf.c   | 136 +++-
 src/conf/nwfilter_conf.h   |  51 +---
 src/conf/virnwfilterobj.c  |   4 +-
 src/libvirt_private.syms   |   7 +-
 src/lxc/lxc_driver.c   |  28 -
 src/nwfilter/nwfilter_driver.c |  22 ++--
 src/nwfilter/nwfilter_gentech_driver.c | 167 -
 src/nwfilter/nwfilter_gentech_driver.h |   4 +-
 src/qemu/qemu_driver.c |  25 
 src/uml/uml_driver.c   |  29 -
 10 files changed, 144 insertions(+), 329 deletions(-)

diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index de26a6d034..706e803a25 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -2819,121 +2819,6 @@ virNWFilterSaveConfig(const char *configDir,
 }
 
 
-int nCallbackDriver;
-#define MAX_CALLBACK_DRIVER 10
-static virNWFilterCallbackDriverPtr callbackDrvArray[MAX_CALLBACK_DRIVER];
-
-void
-virNWFilterRegisterCallbackDriver(virNWFilterCallbackDriverPtr cbd)
-{
-if (nCallbackDriver < MAX_CALLBACK_DRIVER)
-callbackDrvArray[nCallbackDriver++] = cbd;
-}
-
-
-void
-virNWFilterUnRegisterCallbackDriver(virNWFilterCallbackDriverPtr cbd)
-{
-size_t i = 0;
-
-while (i < nCallbackDriver && callbackDrvArray[i] != cbd)
-i++;
-
-if (i < nCallbackDriver) {
-memmove([i], [i+1],
-(nCallbackDriver - i - 1) * sizeof(callbackDrvArray[i]));
-callbackDrvArray[i] = 0;
-nCallbackDriver--;
-}
-}
-
-
-void
-virNWFilterCallbackDriversLock(void)
-{
-size_t i;
-
-for (i = 0; i < nCallbackDriver; i++)
-callbackDrvArray[i]->vmDriverLock();
-}
-
-
-void
-virNWFilterCallbackDriversUnlock(void)
-{
-size_t i;
-
-for (i = 0; i < nCallbackDriver; i++)
-callbackDrvArray[i]->vmDriverUnlock();
-}
-
-
-static virDomainObjListIterator virNWFilterDomainFWUpdateCB;
-static void *virNWFilterDomainFWUpdateOpaque;
-
-/**
- * virNWFilterInstFiltersOnAllVMs:
- * Apply all filters on all running VMs. Don't terminate in case of an
- * error. This should be called upon reloading of the driver.
- */
-int
-virNWFilterInstFiltersOnAllVMs(void)
-{
-size_t i;
-struct domUpdateCBStruct cb = {
-.opaque = virNWFilterDomainFWUpdateOpaque,
-.step = STEP_APPLY_CURRENT,
-.skipInterfaces = NULL, /* not needed */
-};
-
-for (i = 0; i < nCallbackDriver; i++)
-callbackDrvArray[i]->vmFilterRebuild(virNWFilterDomainFWUpdateCB,
- );
-
-return 0;
-}
-
-
-int
-virNWFilterTriggerVMFilterRebuild(void)
-{
-size_t i;
-int ret = 0;
-struct domUpdateCBStruct cb = {
-.opaque = virNWFilterDomainFWUpdateOpaque,
-.step = STEP_APPLY_NEW,
-.skipInterfaces = virHashCreate(0, NULL),
-};
-
-if (!cb.skipInterfaces)
-return -1;
-
-for (i = 0; i < nCallbackDriver; i++) {
-if (callbackDrvArray[i]->vmFilterRebuild(virNWFilterDomainFWUpdateCB,
- ) < 0)
-ret = -1;
-}
-
-if (ret < 0) {
-cb.step = STEP_TEAR_NEW; /* rollback */
-
-for (i = 0; i < nCallbackDriver; i++)
-callbackDrvArray[i]->vmFilterRebuild(virNWFilterDomainFWUpdateCB,
- );
-} else {
-cb.step = STEP_TEAR_OLD; /* switch over */
-
-for (i = 0; i < nCallbackDriver; i++)
-callbackDrvArray[i]->vmFilterRebuild(virNWFilterDomainFWUpdateCB,
- );
-}
-
-virHashFree(cb.skipInterfaces);
-
-return ret;
-}
-
-
 int
 virNWFilterDeleteDef(const char *configDir,
  virNWFilterDefPtr def)
@@ -3204,16 +3089,18 @@ virNWFilterDefFormat(const virNWFilterDef *def)
 return NULL;
 }
 
+static virNWFilterTriggerRebuildCallback rebuildCallback;
+static void *rebuildOpaque;
 
 int
-virNWFilterConfLayerInit(virDomainObjListIterator domUpdateCB,
+virNWFilterConfLayerInit(virNWFilterTriggerRebuildCallback cb,
  void *opaque)
 {
 if (initialized)
 return -1;
 
-virNWFilterDomainFWUpdateCB = domUpdateCB;
-virNWFilterDomainFWUpdateOpaque = opaque;
+rebuildCallback = cb;
+rebuildOpaque = opaque;
 
 initialized = true;
 
@@ -3233,8 +3120,17 @@ virNWFilterConfLayerShutdown(void)
 virRWLockDestroy();
 
 initialized = false;
-virNWFilterDomainFWUpdateOpaque = NULL;
-virNWFilterDomainFWUpdateCB = NULL;
+rebuildCallback 

[libvirt] [PATCH v4 4/6] nwfilter: wire up new APIs for listing and querying filter bindings

[Daniel P . Berrangé]

Wire up the ListAll, LookupByPortDev and GetXMLDesc APIs to allow the
virsh nwfilter-binding-list & nwfilter-binding-dumpxml commands to
work.

Reviewed-by: John Ferlan 
Signed-off-by: Daniel P. Berrangé 
---
 src/nwfilter/nwfilter_driver.c | 76 ++
 1 file changed, 76 insertions(+)

diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index e49e0e7406..79509fc4c0 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -714,6 +714,79 @@ nwfilterTeardownFilter(virDomainNetDefPtr net)
 }
 
 
+static virNWFilterBindingPtr
+nwfilterBindingLookupByPortDev(virConnectPtr conn,
+   const char *portdev)
+{
+virNWFilterBindingPtr ret = NULL;
+virNWFilterBindingObjPtr obj;
+virNWFilterBindingDefPtr def;
+
+obj = virNWFilterBindingObjListFindByPortDev(driver->bindings,
+ portdev);
+if (!obj)
+goto cleanup;
+
+def = virNWFilterBindingObjGetDef(obj);
+if (virNWFilterBindingLookupByPortDevEnsureACL(conn, def) < 0)
+goto cleanup;
+
+ret = virGetNWFilterBinding(conn, def->portdevname, def->filter);
+
+ cleanup:
+virNWFilterBindingObjEndAPI();
+return ret;
+}
+
+
+static int
+nwfilterConnectListAllNWFilterBindings(virConnectPtr conn,
+   virNWFilterBindingPtr **bindings,
+   unsigned int flags)
+{
+int ret;
+
+virCheckFlags(0, -1);
+
+if (virConnectListAllNWFilterBindingsEnsureACL(conn) < 0)
+return -1;
+
+ret = virNWFilterBindingObjListExport(driver->bindings,
+  conn,
+  bindings,
+  
virConnectListAllNWFilterBindingsCheckACL);
+
+return ret;
+}
+
+
+static char *
+nwfilterBindingGetXMLDesc(virNWFilterBindingPtr binding,
+  unsigned int flags)
+{
+virNWFilterBindingObjPtr obj;
+virNWFilterBindingDefPtr def;
+char *ret = NULL;
+
+virCheckFlags(0, NULL);
+
+obj = virNWFilterBindingObjListFindByPortDev(driver->bindings,
+ binding->portdev);
+if (!obj)
+goto cleanup;
+
+def = virNWFilterBindingObjGetDef(obj);
+if (virNWFilterBindingGetXMLDescEnsureACL(binding->conn, def) < 0)
+goto cleanup;
+
+ret = virNWFilterBindingDefFormat(def);
+
+ cleanup:
+virNWFilterBindingObjEndAPI();
+return ret;
+}
+
+
 static virNWFilterDriver nwfilterDriver = {
 .name = "nwfilter",
 .connectNumOfNWFilters = nwfilterConnectNumOfNWFilters, /* 0.8.0 */
@@ -724,6 +797,9 @@ static virNWFilterDriver nwfilterDriver = {
 .nwfilterDefineXML = nwfilterDefineXML, /* 0.8.0 */
 .nwfilterUndefine = nwfilterUndefine, /* 0.8.0 */
 .nwfilterGetXMLDesc = nwfilterGetXMLDesc, /* 0.8.0 */
+.nwfilterBindingLookupByPortDev = nwfilterBindingLookupByPortDev, /* 4.5.0 
*/
+.connectListAllNWFilterBindings = nwfilterConnectListAllNWFilterBindings, 
/* 4.5.0 */
+.nwfilterBindingGetXMLDesc = nwfilterBindingGetXMLDesc, /* 4.5.0 */
 };
 
 
-- 
2.17.1

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

[libvirt] [PATCH v4 5/6] nwfilter: wire up new APIs for creating and deleting nwfilter bindings

[Daniel P . Berrangé]

This allows the virsh commands nwfilter-binding-create and
nwfilter-binding-delete to be used.

Note using these commands lets you delete filters that were
previously created automatically by the virt drivers, or add
filters for VM nics that were not there before. Generally it
is expected these new APIs will only be used by virt drivers.
It is the admin's responsibility to not shoot themselves in
the foot.

Reviewed-by: John Ferlan 
Signed-off-by: Daniel P. Berrangé 
---
 src/nwfilter/nwfilter_driver.c | 86 ++
 1 file changed, 86 insertions(+)

diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index 79509fc4c0..83a2e19dbe 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -787,6 +787,90 @@ nwfilterBindingGetXMLDesc(virNWFilterBindingPtr binding,
 }
 
 
+static virNWFilterBindingPtr
+nwfilterBindingCreateXML(virConnectPtr conn,
+ const char *xml,
+ unsigned int flags)
+{
+virNWFilterBindingObjPtr obj;
+virNWFilterBindingDefPtr def;
+virNWFilterBindingPtr ret = NULL;
+
+virCheckFlags(0, NULL);
+
+def = virNWFilterBindingDefParseString(xml);
+if (!def)
+return NULL;
+
+if (virNWFilterBindingCreateXMLEnsureACL(conn, def) < 0)
+goto cleanup;
+
+obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, 
def->portdevname);
+if (obj) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Filter already present for NIC %s"), 
def->portdevname);
+goto cleanup;
+}
+
+obj = virNWFilterBindingObjListAdd(driver->bindings,
+   def);
+if (!obj)
+goto cleanup;
+
+if (!(ret = virGetNWFilterBinding(conn, def->portdevname, def->filter)))
+goto cleanup;
+
+if (virNWFilterInstantiateFilter(driver, def) < 0) {
+virNWFilterBindingObjListRemove(driver->bindings, obj);
+virObjectUnref(ret);
+ret = NULL;
+goto cleanup;
+}
+virNWFilterBindingObjSave(obj, driver->bindingDir);
+
+ cleanup:
+if (!obj)
+virNWFilterBindingDefFree(def);
+virNWFilterBindingObjEndAPI();
+
+return ret;
+}
+
+
+/*
+ * Note that this is primarily intended for usage by the hypervisor
+ * drivers. it is exposed to the admin, however, and nothing stops
+ * an admin from deleting filter bindings created by the hypervisor
+ * drivers. IOW, it is the admin's responsibility not to shoot
+ * themself in the foot
+ */
+static int
+nwfilterBindingDelete(virNWFilterBindingPtr binding)
+{
+virNWFilterBindingObjPtr obj;
+virNWFilterBindingDefPtr def;
+int ret = -1;
+
+obj = virNWFilterBindingObjListFindByPortDev(driver->bindings, 
binding->portdev);
+if (!obj)
+return -1;
+
+def = virNWFilterBindingObjGetDef(obj);
+if (virNWFilterBindingDeleteEnsureACL(binding->conn, def) < 0)
+goto cleanup;
+
+virNWFilterTeardownFilter(def);
+virNWFilterBindingObjDelete(obj, driver->bindingDir);
+virNWFilterBindingObjListRemove(driver->bindings, obj);
+
+ret = 0;
+
+ cleanup:
+virNWFilterBindingObjEndAPI();
+return ret;
+}
+
+
 static virNWFilterDriver nwfilterDriver = {
 .name = "nwfilter",
 .connectNumOfNWFilters = nwfilterConnectNumOfNWFilters, /* 0.8.0 */
@@ -800,6 +884,8 @@ static virNWFilterDriver nwfilterDriver = {
 .nwfilterBindingLookupByPortDev = nwfilterBindingLookupByPortDev, /* 4.5.0 
*/
 .connectListAllNWFilterBindings = nwfilterConnectListAllNWFilterBindings, 
/* 4.5.0 */
 .nwfilterBindingGetXMLDesc = nwfilterBindingGetXMLDesc, /* 4.5.0 */
+.nwfilterBindingCreateXML = nwfilterBindingCreateXML, /* 4.5.0 */
+.nwfilterBindingDelete = nwfilterBindingDelete, /* 4.5.0 */
 };
 
 
-- 
2.17.1

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

[libvirt] [PATCH v4 6/6] nwfilter: convert virt drivers to use public API for nwfilter bindings

[Daniel P . Berrangé]

Remove the callbacks that the nwfilter driver registers with the domain
object config layer. Instead make the current helper methods call into
the public API for creating/deleting nwfilter bindings.

Signed-off-by: Daniel P. Berrangé 
---
 src/conf/domain_nwfilter.c | 135 +
 src/conf/domain_nwfilter.h |  16 +--
 src/libvirt_private.syms   |   1 -
 src/lxc/lxc_process.c  |   2 +-
 src/nwfilter/nwfilter_driver.c |  82 +++
 src/nwfilter/nwfilter_gentech_driver.c |  42 
 src/nwfilter/nwfilter_gentech_driver.h |   4 -
 src/qemu/qemu_hotplug.c|   4 +-
 src/qemu/qemu_interface.c  |   4 +-
 src/qemu/qemu_process.c|   6 +-
 src/remote/remote_daemon.c |   1 +
 src/uml/uml_conf.c |   2 +-
 12 files changed, 142 insertions(+), 157 deletions(-)

diff --git a/src/conf/domain_nwfilter.c b/src/conf/domain_nwfilter.c
index 7570e0ae83..948b32481e 100644
--- a/src/conf/domain_nwfilter.c
+++ b/src/conf/domain_nwfilter.c
@@ -28,45 +28,146 @@
 #include "datatypes.h"
 #include "domain_conf.h"
 #include "domain_nwfilter.h"
+#include "virnwfilterbindingdef.h"
 #include "virerror.h"
+#include "viralloc.h"
+#include "virstring.h"
+#include "virlog.h"
 
-#define VIR_FROM_THIS VIR_FROM_NWFILTER
 
-static virDomainConfNWFilterDriverPtr nwfilterDriver;
+VIR_LOG_INIT("conf.domain_nwfilter");
 
-void
-virDomainConfNWFilterRegister(virDomainConfNWFilterDriverPtr driver)
+#define VIR_FROM_THIS VIR_FROM_NWFILTER
+
+static virNWFilterBindingDefPtr
+virNWFilterBindingDefForNet(const char *vmname,
+const unsigned char *vmuuid,
+virDomainNetDefPtr net)
 {
-nwfilterDriver = driver;
+virNWFilterBindingDefPtr ret;
+
+if (VIR_ALLOC(ret) < 0)
+return NULL;
+
+if (VIR_STRDUP(ret->ownername, vmname) < 0)
+goto error;
+
+memcpy(ret->owneruuid, vmuuid, sizeof(ret->owneruuid));
+
+if (VIR_STRDUP(ret->portdevname, net->ifname) < 0)
+goto error;
+
+if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT &&
+VIR_STRDUP(ret->linkdevname, net->data.direct.linkdev) < 0)
+goto error;
+
+ret->mac = net->mac;
+
+if (VIR_STRDUP(ret->filter, net->filter) < 0)
+goto error;
+
+if (!(ret->filterparams = virNWFilterHashTableCreate(0)))
+goto error;
+
+if (net->filterparams &&
+virNWFilterHashTablePutAll(net->filterparams, ret->filterparams) < 0)
+goto error;
+
+return ret;
+
+ error:
+virNWFilterBindingDefFree(ret);
+return NULL;
 }
 
+
 int
 virDomainConfNWFilterInstantiate(const char *vmname,
  const unsigned char *vmuuid,
- virDomainNetDefPtr net)
+ virDomainNetDefPtr net,
+ bool ignoreExists)
 {
-if (nwfilterDriver != NULL)
-return nwfilterDriver->instantiateFilter(vmname, vmuuid, net);
+virConnectPtr conn = virGetConnectNWFilter();
+virNWFilterBindingDefPtr def = NULL;
+virNWFilterBindingPtr binding = NULL;
+char *xml;
+int ret = -1;
+
+VIR_DEBUG("vmname=%s portdev=%s filter=%s ignoreExists=%d",
+  vmname, NULLSTR(net->ifname), NULLSTR(net->filter), 
ignoreExists);
+
+if (!conn)
+goto cleanup;
+
+if (ignoreExists) {
+binding = virNWFilterBindingLookupByPortDev(conn, net->ifname);
+if (binding) {
+ret = 0;
+goto cleanup;
+}
+}
 
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-   _("No network filter driver available"));
-return -1;
+if (!(def = virNWFilterBindingDefForNet(vmname, vmuuid, net)))
+goto cleanup;
+
+if (!(xml = virNWFilterBindingDefFormat(def)))
+goto cleanup;
+
+if (!(binding = virNWFilterBindingCreateXML(conn, xml, 0)))
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(xml);
+virNWFilterBindingDefFree(def);
+virObjectUnref(binding);
+virObjectUnref(conn);
+return ret;
 }
 
+
+static void
+virDomainConfNWFilterTeardownImpl(virConnectPtr conn,
+  virDomainNetDefPtr net)
+{
+virNWFilterBindingPtr binding;
+
+binding = virNWFilterBindingLookupByPortDev(conn, net->ifname);
+if (!binding)
+return;
+
+virNWFilterBindingDelete(binding);
+
+virObjectUnref(binding);
+}
+
+
 void
 virDomainConfNWFilterTeardown(virDomainNetDefPtr net)
 {
-if (nwfilterDriver != NULL)
-nwfilterDriver->teardownFilter(net);
+virConnectPtr conn = virGetConnectNWFilter();
+
+if (!conn)
+return;
+
+virDomainConfNWFilterTeardownImpl(conn, net);
+
+virObjectUnref(conn);
 }
 
 void
 virDomainConfVMNWFilterTeardown(virDomainObjPtr vm)
 {
 size_t i;
+virConnectPtr conn = virGetConnectNWFilter();
 
-if 

[libvirt] [PATCH v4 1/6] virsh: add manpage docs for nwfilter-binding commands.

[Daniel P . Berrangé]

Signed-off-by: Daniel P. Berrangé 
---
 tools/virsh.pod | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/tools/virsh.pod b/tools/virsh.pod
index c9ef4f137c..47985ebf78 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -4807,6 +4807,41 @@ variables, and defaults to C.
 
 =back
 
+=head1 NWFILTER BINDING COMMANDS
+
+The following commands manipulate network filter bindings. Network filter
+bindings track the association between a network port and a network
+filter. Generally the bindings are managed automatically by the hypervisor
+drivers when adding/removing NICs on a guest.
+
+If an admin is creating/deleting TAP devices for non-guest usage,
+however, the network filter binding commands provide a way to make use
+of the network filters directly.
+
+=over 4
+
+=item B I
+
+Associate a network port with a network filter. The network filter backend
+will immediately attempt to instantiate the filter rules on the port.
+
+=item B I
+
+Disassociate a network port from a network filter. The network filter
+backend will immediately tear down the filter rules that exist on the
+port.
+
+=item B
+
+List all of the network ports which have filters associated with them
+
+=item B I
+
+Output the network filter binding XML for the network device called
+C
+
+=back
+
 =head1 HYPERVISOR-SPECIFIC COMMANDS
 
 NOTE: Use of the following commands is B discouraged.  They
-- 
2.17.1

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

[libvirt] [PATCH v4 0/6] nwfilter: refactor the driver to make it independent of virt drivers

[Daniel P . Berrangé]

 v1: https://www.redhat.com/archives/libvir-list/2018-April/msg02616.html
 v2: https://www.redhat.com/archives/libvir-list/2018-May/msg01145.html
 v3: https://www.redhat.com/archives/libvir-list/2018-June/msg01121.html

Today the nwfilter driver is entangled with the virt drivers in both
directions. At various times when rebuilding filters nwfilter will call
out to the virt driver to iterate over running guest's NICs. This has
caused very complicated lock ordering rules to be required. If we are to
split the virt drivers out into separate daemons we need to get rid of
this coupling since we don't want the separate daemons calling each
other, as that risks deadlock if all of the RPC workers are busy.

The obvious way to solve this is to have the nwfilter driver remember
all the filters it has active, avoiding the need to iterate over running
guests.

Changed in v4:

 - Merged already reviewed patches
 - Correctly handle nwfilter recreate on daemon startup
 - Added virsh docs

Changed in v3:

  - Updated API version numbers
  - Use accessors for virNWFilterBindingObjPtr struct
  - Other fixes John mentioned

Changed in v2:

 - The virNWFilterBindingPtr was renamed virNWFilterBindingDefPtr
 - New virNWFilterBindingObjPtr & virNWFilterBindingObjListPtr
   structs added to track the objects in the driver
 - New virNWFilterBindingPtr  public API type was added
 - New public APIs for listing filter bindings, querying XML, and
   creating/deleting them
 - Convert the virt drivers to use the public API for creating
   and deleting bindings
 - Persistent active bindings out to disk so they're preserved
   across restarts
 - Added RNG schema and XML-2-XML test
 - New virsh commands for listing/querying XML/creating/deleting
   bindings

Daniel P. Berrangé (6):
  virsh: add manpage docs for nwfilter-binding commands.
  nwfilter: keep track of active filter bindings
  nwfilter: remove virt driver callback layer for rebuilding filters
  nwfilter: wire up new APIs for listing and querying filter bindings
  nwfilter: wire up new APIs for creating and deleting nwfilter bindings
  nwfilter: convert virt drivers to use public API for nwfilter bindings

 src/conf/domain_nwfilter.c | 135 --
 src/conf/domain_nwfilter.h |  16 +-
 src/conf/nwfilter_conf.c   | 136 ++
 src/conf/nwfilter_conf.h   |  51 +-
 src/conf/virnwfilterobj.c  |   4 +-
 src/conf/virnwfilterobj.h  |   4 +
 src/libvirt_private.syms   |   8 +-
 src/lxc/lxc_driver.c   |  28 ---
 src/lxc/lxc_process.c  |   2 +-
 src/nwfilter/nwfilter_driver.c | 236 -
 src/nwfilter/nwfilter_gentech_driver.c | 187 ++--
 src/nwfilter/nwfilter_gentech_driver.h |   8 +-
 src/qemu/qemu_driver.c |  25 ---
 src/qemu/qemu_hotplug.c|   4 +-
 src/qemu/qemu_interface.c  |   4 +-
 src/qemu/qemu_process.c|   6 +-
 src/remote/remote_daemon.c |   1 +
 src/uml/uml_conf.c |   2 +-
 src/uml/uml_driver.c   |  29 ---
 tools/virsh.pod|  35 
 20 files changed, 478 insertions(+), 443 deletions(-)

-- 
2.17.1

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

[libvirt] [PATCH v2 1/3] qemuDomainUpdateDeviceFlags: Parse device as live if needed

[Michal Privoznik]

When updating device it's worth parsing live info too as users
might want to update it as well.

Signed-off-by: Michal Privoznik 

Reviewed-by: John Ferlan 
Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_driver.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 129bacdd34..e3fb4919a5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8631,7 +8631,7 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
 int ret = -1;
 virQEMUDriverConfigPtr cfg = NULL;
 virCapsPtr caps = NULL;
-unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE;
+unsigned int parse_flags = 0;
 
 virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
   VIR_DOMAIN_AFFECT_CONFIG |
@@ -8653,15 +8653,19 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
 if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
 goto cleanup;
 
+if (virDomainObjUpdateModificationImpact(vm, ) < 0)
+goto endjob;
+
+if ((flags & VIR_DOMAIN_AFFECT_CONFIG) &&
+!(flags & VIR_DOMAIN_AFFECT_LIVE))
+parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE;
+
 dev = dev_copy = virDomainDeviceDefParse(xml, vm->def,
  caps, driver->xmlopt,
  parse_flags);
 if (dev == NULL)
 goto endjob;
 
-if (virDomainObjUpdateModificationImpact(vm, ) < 0)
-goto endjob;
-
 if (flags & VIR_DOMAIN_AFFECT_CONFIG &&
 flags & VIR_DOMAIN_AFFECT_LIVE) {
 /* If we are affecting both CONFIG and LIVE
-- 
2.16.4

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

[libvirt] [PATCH v3 0/3] Implement the HTM pSeries feature

[Andrea Bolognani]

Applies cleanly on top of aa51063927a0d48768ff88b11f12075ad5efc89f.

Changes from [v2]:

  * rebase on top of master; the series is much smaller now.

Changes from [v1]:

  * drop qom-list-properties machinery, as equivalent functionality
has been merged in the meantime;
  * don't introduce scaffolding for uniform machine option handling
as a requirement for implementing this specific feature: we can
do ad-hoc processing for now, per the status quo, then clean up
everything (including existing features) with a separate series
later.

Changes from [RFC v3]:

  * can be considered for merging, since the QEMU part already has;
  * fix compatibility issues with QEMU <2.12 spotted by Shivaprasad;
  * drop all features except for HTM, at least for the time being;
  * add documentation.

Changes from [RFC v2]:

  * use qom-list-properties to probe availability;
  * test all features with a single XML file.

Changes from [RFC v1]:

  * don't nest features inside a  element;
  * implement all optional features.

[v2] https://www.redhat.com/archives/libvir-list/2018-June/msg01374.html
[v1] https://www.redhat.com/archives/libvir-list/2018-March/msg00474.html
[RFC v3] https://www.redhat.com/archives/libvir-list/2018-March/msg00042.html
[RFC v2] https://www.redhat.com/archives/libvir-list/2018-February/msg00310.html
[RFC v1] https://www.redhat.com/archives/libvir-list/2018-January/msg00779.html

Andrea Bolognani (3):
  qemu: Add capability for the HTM pSeries feature
  qemu: Implement the HTM pSeries feature
  news: Update for the HTM pSeries feature

 docs/formatdomain.html.in |  8 
 docs/news.xml |  9 +
 docs/schemas/domaincommon.rng |  5 +
 src/conf/domain_conf.c| 19 ++
 src/conf/domain_conf.h|  1 +
 src/qemu/qemu_capabilities.c  |  2 ++
 src/qemu/qemu_capabilities.h  |  1 +
 src/qemu/qemu_command.c   | 20 +++
 src/qemu/qemu_domain.c| 13 
 .../caps_2.12.0.ppc64.xml |  1 +
 .../qemucapabilitiesdata/caps_3.0.0.ppc64.xml |  1 +
 tests/qemuxml2argvdata/pseries-features.args  |  2 +-
 tests/qemuxml2argvdata/pseries-features.xml   |  1 +
 tests/qemuxml2argvtest.c  |  1 +
 tests/qemuxml2xmloutdata/pseries-features.xml |  1 +
 tests/qemuxml2xmltest.c   |  1 +
 16 files changed, 85 insertions(+), 1 deletion(-)

-- 
2.17.1

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

[libvirt] [PATCH v2 0/3] Deny live device alias change

[Michal Privoznik]

v2 of:

https://www.redhat.com/archives/libvir-list/2018-June/msg00971.html

Note that 1/3 is already reviewed (not pushed though, yet).

diff to v1:
- Fixed  hotplug issue raised during review of v1,
- Removed alias change checks for disks and interfaces since there's one
  common place for that now.

Michal Privoznik (3):
  qemuDomainUpdateDeviceFlags: Parse device as live if needed
  conf: Reintroduce action to virDomainDefCompatibleDevice
  conf: Forbid device alias change on device-update

 src/conf/domain_conf.c  | 14 +-
 src/conf/domain_conf.h  |  4 +++-
 src/lxc/lxc_driver.c| 12 +---
 src/qemu/qemu_domain.c  |  8 
 src/qemu/qemu_driver.c  | 44 
 src/qemu/qemu_hotplug.c |  5 -
 6 files changed, 57 insertions(+), 30 deletions(-)

-- 
2.16.4

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

[libvirt] [PATCH v3 3/3] news: Update for the HTM pSeries feature

[Andrea Bolognani]

Signed-off-by: Andrea Bolognani 
---
 docs/news.xml | 9 +
 1 file changed, 9 insertions(+)

diff --git a/docs/news.xml b/docs/news.xml
index a32d2b88a5..51e09a8737 100644
--- a/docs/news.xml
+++ b/docs/news.xml
@@ -105,6 +105,15 @@
   guest.
 
   
+  
+
+  qemu: Implement the HTM pSeries feature
+
+
+  Users can now decide whether HTM (Hardware Transactional Memory)
+  support should be available to the guest.
+
+  
 
 
   
-- 
2.17.1

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

[libvirt] [PATCH v3 2/3] qemu: Implement the HTM pSeries feature

[Andrea Bolognani]

https://bugzilla.redhat.com/show_bug.cgi?id=1525599
Signed-off-by: Andrea Bolognani 
---
 docs/formatdomain.html.in |  8 
 docs/schemas/domaincommon.rng |  5 +
 src/conf/domain_conf.c| 19 ++
 src/conf/domain_conf.h|  1 +
 src/qemu/qemu_command.c   | 20 +++
 src/qemu/qemu_domain.c| 13 
 tests/qemuxml2argvdata/pseries-features.args  |  2 +-
 tests/qemuxml2argvdata/pseries-features.xml   |  1 +
 tests/qemuxml2argvtest.c  |  1 +
 tests/qemuxml2xmloutdata/pseries-features.xml |  1 +
 tests/qemuxml2xmltest.c   |  1 +
 11 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 0d68596991..73db315c4f 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -1929,6 +1929,7 @@
   smm state='on'
 tseg unit='MiB'48/tseg
   /smm
+  htm state='on'/
 /features
 ...
 
@@ -2162,6 +2163,13 @@
   Enable QEMU vmcoreinfo device to let the guest kernel save debug
   details. Since 4.4.0 (QEMU only)
   
+  htm
+  Configure HTM (Hardware Transational Memory) availability for
+  pSeries guests. Possible values for the state attribute
+  are on and off. If the attribute is not
+  defined, the hypervisor default will be used.
+  Since 4.5.0 (QEMU/KVM only)
+  
 
 
 Time keeping
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 450e8ac77b..3dd1bd7974 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4906,6 +4906,11 @@
   
 
   
+  
+
+  
+
+  
 
   
 
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d8cb7f37f3..ea01d08a3c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -152,6 +152,7 @@ VIR_ENUM_IMPL(virDomainFeature, VIR_DOMAIN_FEATURE_LAST,
   "ioapic",
   "hpt",
   "vmcoreinfo",
+  "htm",
 );
 
 VIR_ENUM_IMPL(virDomainCapabilitiesPolicy, VIR_DOMAIN_CAPABILITIES_POLICY_LAST,
@@ -19827,6 +19828,22 @@ virDomainDefParseXML(xmlDocPtr xml,
 }
 break;
 
+case VIR_DOMAIN_FEATURE_HTM:
+if (!(tmp = virXMLPropString(nodes[i], "state"))) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("missing state attribute '%s' of feature 
'%s'"),
+   tmp, virDomainFeatureTypeToString(val));
+goto error;
+}
+if ((def->features[val] = virTristateSwitchTypeFromString(tmp)) < 
0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("unknown state attribute '%s' of feature 
'%s'"),
+   tmp, virDomainFeatureTypeToString(val));
+goto error;
+}
+VIR_FREE(tmp);
+break;
+
 /* coverity[dead_error_begin] */
 case VIR_DOMAIN_FEATURE_LAST:
 break;
@@ -21961,6 +21978,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr 
src,
 case VIR_DOMAIN_FEATURE_VMPORT:
 case VIR_DOMAIN_FEATURE_SMM:
 case VIR_DOMAIN_FEATURE_VMCOREINFO:
+case VIR_DOMAIN_FEATURE_HTM:
 if (src->features[i] != dst->features[i]) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("State of feature '%s' differs: "
@@ -27626,6 +27644,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
 case VIR_DOMAIN_FEATURE_PMU:
 case VIR_DOMAIN_FEATURE_PVSPINLOCK:
 case VIR_DOMAIN_FEATURE_VMPORT:
+case VIR_DOMAIN_FEATURE_HTM:
 switch ((virTristateSwitch) def->features[i]) {
 case VIR_TRISTATE_SWITCH_LAST:
 case VIR_TRISTATE_SWITCH_ABSENT:
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0924fc4f3c..a41431ccf9 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1771,6 +1771,7 @@ typedef enum {
 VIR_DOMAIN_FEATURE_IOAPIC,
 VIR_DOMAIN_FEATURE_HPT,
 VIR_DOMAIN_FEATURE_VMCOREINFO,
+VIR_DOMAIN_FEATURE_HTM,
 
 VIR_DOMAIN_FEATURE_LAST
 } virDomainFeature;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a357d2199c..5342c08f19 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7291,6 +7291,26 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
 }
 }
 
+if (def->features[VIR_DOMAIN_FEATURE_HTM] != VIR_TRISTATE_SWITCH_ABSENT) {
+const char *str;
+
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_PSERIES_CAP_HTM)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   

[libvirt] [PATCH v2 3/3] conf: Forbid device alias change on device-update

[Michal Privoznik]

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

When updating a live device users might pass different alias than
the one the device has. Currently, this is silently ignored which
goes against our behaviour for other parts of the device where we
explicitly allow only certain changes and error out loudly on
anything else.

Signed-off-by: Michal Privoznik 
---
 src/conf/domain_conf.c  | 13 -
 src/conf/domain_conf.h  |  3 ++-
 src/lxc/lxc_driver.c|  9 ++---
 src/qemu/qemu_domain.c  |  8 
 src/qemu/qemu_driver.c  | 24 
 src/qemu/qemu_hotplug.c |  5 -
 6 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 93cfca351c..b8b53450fa 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -28206,7 +28206,8 @@ int
 virDomainDefCompatibleDevice(virDomainDefPtr def,
  virDomainDeviceDefPtr dev,
  virDomainDeviceDefPtr oldDev,
- virDomainDeviceAction action ATTRIBUTE_UNUSED)
+ virDomainDeviceAction action,
+ bool live)
 {
 virDomainCompatibleDeviceData data = {
 .newInfo = virDomainDeviceGetInfo(dev),
@@ -28216,6 +28217,16 @@ virDomainDefCompatibleDevice(virDomainDefPtr def,
 if (oldDev)
 data.oldInfo = virDomainDeviceGetInfo(oldDev);
 
+if (action == VIR_DOMAIN_DEVICE_ACTION_UPDATE &&
+live &&
+((!!data.newInfo != !!data.oldInfo) ||
+ (data.newInfo && data.oldInfo &&
+  STRNEQ_NULLABLE(data.newInfo->alias, data.oldInfo->alias {
+virReportError(VIR_ERR_OPERATION_DENIED, "%s",
+   _("changing device alias is not allowed"));
+return -1;
+}
+
 if (!virDomainDefHasUSB(def) &&
 def->os.type != VIR_DOMAIN_OSTYPE_EXE &&
 virDomainDeviceIsUSB(dev)) {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index f33405e097..71437dc485 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3107,7 +3107,8 @@ typedef enum {
 int virDomainDefCompatibleDevice(virDomainDefPtr def,
  virDomainDeviceDefPtr dev,
  virDomainDeviceDefPtr oldDev,
- virDomainDeviceAction action);
+ virDomainDeviceAction action,
+ bool live);
 
 void virDomainRNGDefFree(virDomainRNGDefPtr def);
 
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 850b12726b..8c02f888f4 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -3550,7 +3550,8 @@ lxcDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
 
 oldDev.data.net = vmdef->nets[idx];
 if (virDomainDefCompatibleDevice(vmdef, dev, ,
- VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0)
+ VIR_DOMAIN_DEVICE_ACTION_UPDATE,
+ false) < 0)
 return -1;
 
 virDomainNetDefFree(vmdef->nets[idx]);
@@ -4787,7 +4788,8 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom,
 goto endjob;
 
 if (virDomainDefCompatibleDevice(vmdef, dev, NULL,
- VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0)
+ VIR_DOMAIN_DEVICE_ACTION_ATTACH,
+ false) < 0)
 goto endjob;
 
 if ((ret = lxcDomainAttachDeviceConfig(vmdef, dev)) < 0)
@@ -4796,7 +4798,8 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom,
 
 if (flags & VIR_DOMAIN_AFFECT_LIVE) {
 if (virDomainDefCompatibleDevice(vm->def, dev_copy, NULL,
- VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0)
+ VIR_DOMAIN_DEVICE_ACTION_ATTACH,
+ true) < 0)
 goto endjob;
 
 if ((ret = lxcDomainAttachDeviceLive(dom->conn, driver, vm, dev_copy)) 
< 0)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 6d203e1f2e..d750f3382a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8613,14 +8613,6 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
 return false;
 }
 
-if (disk->info.alias &&
-STRNEQ_NULLABLE(disk->info.alias, orig_disk->info.alias)) {
-virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
-   _("cannot modify field '%s' of the disk"),
-   "alias");
-return false;
-}
-
 CHECK_EQ(info.bootIndex, "boot order", true);
 CHECK_EQ(rawio, "rawio", true);
 CHECK_EQ(sgio, "sgio", true);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8c0c681c4d..21c97feed8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7886,7 +7886,8 @@ 

[libvirt] [PATCH v2 2/3] conf: Reintroduce action to virDomainDefCompatibleDevice

[Michal Privoznik]

This was lost in c57f3fd2f8999d17e01. But now we are going to
need it again.

Signed-off-by: Michal Privoznik 
---
 src/conf/domain_conf.c |  3 ++-
 src/conf/domain_conf.h |  3 ++-
 src/lxc/lxc_driver.c   |  9 ++---
 src/qemu/qemu_driver.c | 24 
 4 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d8cb7f37f3..93cfca351c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -28205,7 +28205,8 @@ virDomainDeviceInfoCheckBootIndex(virDomainDefPtr def 
ATTRIBUTE_UNUSED,
 int
 virDomainDefCompatibleDevice(virDomainDefPtr def,
  virDomainDeviceDefPtr dev,
- virDomainDeviceDefPtr oldDev)
+ virDomainDeviceDefPtr oldDev,
+ virDomainDeviceAction action ATTRIBUTE_UNUSED)
 {
 virDomainCompatibleDeviceData data = {
 .newInfo = virDomainDeviceGetInfo(dev),
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0924fc4f3c..f33405e097 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3106,7 +3106,8 @@ typedef enum {
 
 int virDomainDefCompatibleDevice(virDomainDefPtr def,
  virDomainDeviceDefPtr dev,
- virDomainDeviceDefPtr oldDev);
+ virDomainDeviceDefPtr oldDev,
+ virDomainDeviceAction action);
 
 void virDomainRNGDefFree(virDomainRNGDefPtr def);
 
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index cfb431488d..850b12726b 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -3549,7 +3549,8 @@ lxcDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
 goto cleanup;
 
 oldDev.data.net = vmdef->nets[idx];
-if (virDomainDefCompatibleDevice(vmdef, dev, ) < 0)
+if (virDomainDefCompatibleDevice(vmdef, dev, ,
+ VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0)
 return -1;
 
 virDomainNetDefFree(vmdef->nets[idx]);
@@ -4785,7 +4786,8 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom,
 if (!vmdef)
 goto endjob;
 
-if (virDomainDefCompatibleDevice(vmdef, dev, NULL) < 0)
+if (virDomainDefCompatibleDevice(vmdef, dev, NULL,
+ VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0)
 goto endjob;
 
 if ((ret = lxcDomainAttachDeviceConfig(vmdef, dev)) < 0)
@@ -4793,7 +4795,8 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom,
 }
 
 if (flags & VIR_DOMAIN_AFFECT_LIVE) {
-if (virDomainDefCompatibleDevice(vm->def, dev_copy, NULL) < 0)
+if (virDomainDefCompatibleDevice(vm->def, dev_copy, NULL,
+ VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0)
 goto endjob;
 
 if ((ret = lxcDomainAttachDeviceLive(dom->conn, driver, vm, dev_copy)) 
< 0)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e3fb4919a5..8c0c681c4d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7885,7 +7885,8 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm,
 }
 
 oldDev.data.disk = orig_disk;
-if (virDomainDefCompatibleDevice(vm->def, dev, ) < 0)
+if (virDomainDefCompatibleDevice(vm->def, dev, ,
+ VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0)
 goto cleanup;
 
 if (!qemuDomainDiskChangeSupported(disk, orig_disk))
@@ -7943,7 +7944,8 @@ qemuDomainUpdateDeviceLive(virDomainObjPtr vm,
 case VIR_DOMAIN_DEVICE_GRAPHICS:
 if ((idx = qemuDomainFindGraphicsIndex(vm->def, dev->data.graphics)) 
>= 0) {
 oldDev.data.graphics = vm->def->graphics[idx];
-if (virDomainDefCompatibleDevice(vm->def, dev, ) < 0)
+if (virDomainDefCompatibleDevice(vm->def, dev, ,
+ VIR_DOMAIN_DEVICE_ACTION_UPDATE) 
< 0)
 return -1;
 }
 
@@ -7953,7 +7955,8 @@ qemuDomainUpdateDeviceLive(virDomainObjPtr vm,
 case VIR_DOMAIN_DEVICE_NET:
 if ((idx = virDomainNetFindIdx(vm->def, dev->data.net)) >= 0) {
 oldDev.data.net = vm->def->nets[idx];
-if (virDomainDefCompatibleDevice(vm->def, dev, ) < 0)
+if (virDomainDefCompatibleDevice(vm->def, dev, ,
+ VIR_DOMAIN_DEVICE_ACTION_UPDATE) 
< 0)
 return -1;
 }
 
@@ -8406,7 +8409,8 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
 }
 
 oldDev.data.disk = vmdef->disks[pos];
-if (virDomainDefCompatibleDevice(vmdef, dev, ) < 0)
+if (virDomainDefCompatibleDevice(vmdef, dev, ,
+ VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0)
 return -1;
 
 virDomainDiskDefFree(vmdef->disks[pos]);
@@ -8425,7 +8429,8 @@ 

[libvirt] [PATCH v3 1/3] qemu: Add capability for the HTM pSeries feature

[Andrea Bolognani]

Signed-off-by: Andrea Bolognani 
---
 src/qemu/qemu_capabilities.c | 2 ++
 src/qemu/qemu_capabilities.h | 1 +
 tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 +
 tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml  | 1 +
 4 files changed, 5 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 37c8fbe3d3..c7da916f9a 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -501,6 +501,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   /* 310 */
   "sev-guest",
   "machine.pseries.cap-hpt-max-page-size",
+  "machine.pseries.cap-htm",
 );
 
 
@@ -1431,6 +1432,7 @@ static struct virQEMUCapsStringFlags 
virQEMUCapsObjectPropsMemoryBackendFile[] =
 
 static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsSPAPRMachine[] = {
 { "cap-hpt-max-page-size", QEMU_CAPS_MACHINE_PSERIES_CAP_HPT_MAX_PAGE_SIZE 
},
+{ "cap-htm", QEMU_CAPS_MACHINE_PSERIES_CAP_HTM },
 };
 
 static virQEMUCapsObjectTypeProps virQEMUCapsObjectProps[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index df9bf49abb..a048a1cf02 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -485,6 +485,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for 
syntax-check */
 /* 310 */
 QEMU_CAPS_SEV_GUEST, /* -object sev-guest,... */
 QEMU_CAPS_MACHINE_PSERIES_CAP_HPT_MAX_PAGE_SIZE, /* -machine 
pseries.cap-hpt-max-page-size */
+QEMU_CAPS_MACHINE_PSERIES_CAP_HTM, /* -machine pseries.cap-htm */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
index 2ee582f343..7139179304 100644
--- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml
@@ -166,6 +166,7 @@
   
   
   
+  
   2011090
   0
   428334
diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml 
b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml
index 7e958b2efc..33cd00e613 100644
--- a/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml
@@ -166,6 +166,7 @@
   
   
   
+  
   2012050
   0
   446771
-- 
2.17.1

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

[libvirt] [python PATCH] Add support for nwfilter binding objects / apis

[Daniel P . Berrangé]

Signed-off-by: Daniel P. Berrangé 
---
 generator.py   | 33 --
 libvirt-override-api.xml   |  6 
 libvirt-override-virConnect.py | 12 
 libvirt-override.c | 51 ++
 typewrappers.c | 13 +
 typewrappers.h | 10 +++
 6 files changed, 122 insertions(+), 3 deletions(-)

diff --git a/generator.py b/generator.py
index 353adab..ae0e45c 100755
--- a/generator.py
+++ b/generator.py
@@ -356,6 +356,10 @@ py_types = {
 'virNWFilter *':  ('O', "virNWFilter", "virNWFilterPtr", "virNWFilterPtr"),
 'const virNWFilter *':  ('O', "virNWFilter", "virNWFilterPtr", 
"virNWFilterPtr"),
 
+'virNWFilterBindingPtr':  ('O', "virNWFilterBinding", 
"virNWFilterBindingPtr", "virNWFilterBindingPtr"),
+'virNWFilterBinding *':  ('O', "virNWFilterBinding", 
"virNWFilterBindingPtr", "virNWFilterBindingPtr"),
+'const virNWFilterBinding *':  ('O', "virNWFilterBinding", 
"virNWFilterBindingPtr", "virNWFilterBindingPtr"),
+
 'virStreamPtr':  ('O', "virStream", "virStreamPtr", "virStreamPtr"),
 'virStream *':  ('O', "virStream", "virStreamPtr", "virStreamPtr"),
 'const virStream *':  ('O', "virStream", "virStreamPtr", "virStreamPtr"),
@@ -539,6 +543,7 @@ skip_function = (
 'virConnectListAllInterfaces', # overridden in virConnect.py
 'virConnectListAllNodeDevices', # overridden in virConnect.py
 'virConnectListAllNWFilters', # overridden in virConnect.py
+'virConnectListAllNWFilterBindings', # overridden in virConnect.py
 'virConnectListAllSecrets', # overridden in virConnect.py
 'virConnectGetAllDomainStats', # overridden in virConnect.py
 'virDomainListGetStats', # overriden in virConnect.py
@@ -572,6 +577,7 @@ skip_function = (
 "virNodeDeviceRef",
 "virSecretRef",
 "virNWFilterRef",
+"virNWFilterBindingRef",
 "virStoragePoolRef",
 "virStorageVolRef",
 "virStreamRef",
@@ -1010,6 +1016,8 @@ classes_type = {
 "virSecret *": ("._o", "virSecret(self, _obj=%s)", "virSecret"),
 "virNWFilterPtr": ("._o", "virNWFilter(self, _obj=%s)", "virNWFilter"),
 "virNWFilter *": ("._o", "virNWFilter(self, _obj=%s)", "virNWFilter"),
+"virNWFilterBindingPtr": ("._o", "virNWFilterBinding(self, _obj=%s)", 
"virNWFilterBinding"),
+"virNWFilterBinding *": ("._o", "virNWFilterBinding(self, _obj=%s)", 
"virNWFilterBinding"),
 "virStreamPtr": ("._o", "virStream(self, _obj=%s)", "virStream"),
 "virStream *": ("._o", "virStream(self, _obj=%s)", "virStream"),
 "virConnectPtr": ("._o", "virConnect(_obj=%s)", "virConnect"),
@@ -1021,7 +1029,8 @@ classes_type = {
 primary_classes = ["virDomain", "virNetwork", "virInterface",
"virStoragePool", "virStorageVol",
"virConnect", "virNodeDevice", "virSecret",
-   "virNWFilter", "virStream", "virDomainSnapshot"]
+   "virNWFilter", "virNWFilterBinding",
+   "virStream", "virDomainSnapshot"]
 
 classes_destructors = {
 "virDomain": "virDomainFree",
@@ -1032,6 +1041,7 @@ classes_destructors = {
 "virNodeDevice" : "virNodeDeviceFree",
 "virSecret": "virSecretFree",
 "virNWFilter": "virNWFilterFree",
+"virNWFilterBinding": "virNWFilterBindingFree",
 "virDomainSnapshot": "virDomainSnapshotFree",
 # We hand-craft __del__ for this one
 #"virStream": "virStreamFree",
@@ -1058,6 +1068,8 @@ functions_noexcept = {
 'virSecretGetUsageType': True,
 'virSecretGetUsageID': True,
 'virNWFilterGetName': True,
+'virNWFilterBindingGetFilterName': True,
+'virNWFilterBindingGetPortDev': True,
 }
 
 function_classes = {}
@@ -1113,6 +1125,15 @@ def nameFixup(name, classe, type, file):
 elif name[0:15] == "virSecretLookup":
 func = name[3:]
 func = func[0:1].lower() + func[1:]
+elif name[0:27] == "virNWFilterBindingCreateXML":
+func = name[3:]
+func = func[0:1].lower() + func[1:]
+elif name[0:24] == "virNWFilterBindingDefine":
+func = name[3:]
+func = func[0:3].lower() + func[3:]
+elif name[0:24] == "virNWFilterBindingLookup":
+func = name[3:]
+func = func[0:3].lower() + func[3:]
 elif name[0:17] == "virNWFilterDefine":
 func = name[3:]
 func = func[0:3].lower() + func[3:]
@@ -1189,6 +1210,12 @@ def nameFixup(name, classe, type, file):
 elif name[0:9] == 'virSecret':
 func = name[9:]
 func = func[0:1].lower() + func[1:]
+elif name[0:21] == 'virNWFilterBindingGet':
+func = name[21:]
+func = func[0:1].lower() + func[1:]
+elif name[0:18] == 'virNWFilterBinding':
+func = name[18:]
+func = func[0:1].lower() + func[1:]
 elif name[0:14] == 'virNWFilterGet':
 func = name[14:]
 func = func[0:1].lower() + func[1:]
@@ -1468,7 +1495,7 @@ def buildWrappers(module):
 classes.write("class 

Re: [libvirt] [PATCH 3/7] conf: Reintroduce virDomainDef::hpt_resizing

[Michal Privoznik]

On 06/26/2018 10:19 AM, Andrea Bolognani wrote:
> On Tue, 2018-06-26 at 09:18 +0200, Michal Privoznik wrote:
>> On 06/25/2018 07:11 PM, Andrea Bolognani wrote:
>>> @@ -19803,9 +19803,12 @@ virDomainDefParseXML(xmlDocPtr xml,
>>> tmp);
>>>  goto error;
>>>  }
>>> -def->features[val] = value;
>>> +def->hpt_resizing = (virDomainHPTResizing) value;
>>
>> This typecast seems useless. Also, pre-existing, the if() statement
>> above is more verbose than it needs to be since
>> VIR_DOMAIN_HPT_RESIZING_NONE is defined to be zero.
> 
> I agree the cast is mostly unnecessary here, but it doesn't cause
> any issues either and as a personal preference I like being explicit
> about this kind of conversion :)
> 
> As for the if() statement above, which looks like
> 
>   int value = virDomainHPTResizingTypeFromString(tmp);
>   if (value < 0 || value == VIR_DOMAIN_HPT_RESIZING_NONE) {
>   ...
> 
> you could definitely rewrite the condition as 'value <= 0' and get
> the same behavior, but you would make the code more opaque as a
> result.
> 
> Right now it doesn't take much effort to figure out what the code
> above does: it very obviouly tries to convert a string to an enum,
> and errors out if either the conversion fails entirely or the
> returned value is one that we don't want the user to specify.
> 
> If rewritten, the two error conditions would be smushed together
> and the developer would need to unpack them in their head, possibly
> after looking up the virDomainHPTResizing enum and confirming 0
> corresponds to a value that it would probably make sense to
> blacklist when parsing the configuration.
> 
> The trade-off is very much not worth it just to save a few dozen
> characters, in my opinion.
> 

The thing is, we are not consistent. On many places we use short version
and in fewer places (my feeling, haven't counted them) we are using this
explicit longer version.
Also, usually the enums are designed that way so the first element (if
set to zero) means 'nada', unset value so that we can do checks like
this later in the code:

if (def->enum == 0) { /* unset */ }

So by looking at the code I can usually tell if the first element is
supposed to be accepted or not:

if (value < 0) ...
if (value <= 0) ...

Either works, it's just that we ought to be consistent. But since you're
not touching that line anyway, it's okay to leave it as is and say
'pre-existing :-P'.

Michal

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

Re: [libvirt] [PATCH] set-lifecycle-action: add description of type and action

[Chen Hanxiao]

At 2018-06-21 19:28:55, "Chen Hanxiao"  wrote:
>From: Chen Hanxiao 
>
>In [1],  are described as "on_poweroff",
>   "on_reboot", "on_crash".
>   but we accept "poweroff", "reboot" and "crash".
>This patch adds docs about them.
>
>[1]: https://libvirt.org/formatdomain.html#elementsEvents
>
>Signed-off-by: Chen Hanxiao 
>---

ping

Regards,
- Chen

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

Re: [libvirt] [PATCH 0/7] qemu: Support page size tuning for pSeries guests

[Andrea Bolognani]

On Tue, 2018-06-26 at 09:18 +0200, Michal Privoznik wrote:
> On 06/25/2018 07:11 PM, Andrea Bolognani wrote:
> > This applies cleanly on top of a0d6894af1b1.
> > 
> > Patch 1/7 is too big to go through the list; it can be fetched,
> > along with the rest of the series, from [1].
[...]
> > [1] https://github.com/andreabolognani/libvirt/tree/pseries-hpt-maxpagesize
> 
> would be nice if the branch would be derived from the master you have
> there too ;-)

Whoops, looks like I forgot to push the master branch... I even had
some garbage there. Sorry about that.

> ACK

Pushed, thanks :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH 3/7] conf: Reintroduce virDomainDef::hpt_resizing

[Andrea Bolognani]

On Tue, 2018-06-26 at 09:18 +0200, Michal Privoznik wrote:
> On 06/25/2018 07:11 PM, Andrea Bolognani wrote:
> > @@ -19803,9 +19803,12 @@ virDomainDefParseXML(xmlDocPtr xml,
> > tmp);
> >  goto error;
> >  }
> > -def->features[val] = value;
> > +def->hpt_resizing = (virDomainHPTResizing) value;
> 
> This typecast seems useless. Also, pre-existing, the if() statement
> above is more verbose than it needs to be since
> VIR_DOMAIN_HPT_RESIZING_NONE is defined to be zero.

I agree the cast is mostly unnecessary here, but it doesn't cause
any issues either and as a personal preference I like being explicit
about this kind of conversion :)

As for the if() statement above, which looks like

  int value = virDomainHPTResizingTypeFromString(tmp);
  if (value < 0 || value == VIR_DOMAIN_HPT_RESIZING_NONE) {
  ...

you could definitely rewrite the condition as 'value <= 0' and get
the same behavior, but you would make the code more opaque as a
result.

Right now it doesn't take much effort to figure out what the code
above does: it very obviouly tries to convert a string to an enum,
and errors out if either the conversion fails entirely or the
returned value is one that we don't want the user to specify.

If rewritten, the two error conditions would be smushed together
and the developer would need to unpack them in their head, possibly
after looking up the virDomainHPTResizing enum and confirming 0
corresponds to a value that it would probably make sense to
blacklist when parsing the configuration.

The trade-off is very much not worth it just to save a few dozen
characters, in my opinion.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [jenkins-ci PATCH v2] jobs: Change git URL to fast external mirrors

[Daniel P . Berrangé]

On Tue, Jun 26, 2018 at 09:30:32AM +0200, Pavel Hrdina wrote:
> We had our own local copy of all projects synchronized by cron on
> the host where we have the CI VMs.  This was to save the traffic from
> libvirt.org repositories and to make the cloning for our Jenkins jobs
> faster.
> 
> We might move our VMs into a cloud in future we would not be able to
> have any local copy so this changes the git URL to use fast external
> mirrors.
> 
> Signed-off-by: Pavel Hrdina 
> ---
> 
> v1: 
> 
>  jobs/autotools.yaml   | 2 +-
>  jobs/generic.yaml | 2 +-
>  jobs/go.yaml  | 2 +-
>  jobs/perl-modulebuild.yaml| 2 +-
>  jobs/python-distutils.yaml| 2 +-
>  projects/libosinfo.yaml   | 1 +
>  projects/libvirt-cim.yaml | 1 +
>  projects/libvirt-dbus.yaml| 1 +
>  projects/libvirt-glib.yaml| 1 +
>  projects/libvirt-go-xml.yaml  | 1 +
>  projects/libvirt-go.yaml  | 1 +
>  projects/libvirt-perl.yaml| 1 +
>  projects/libvirt-python.yaml  | 1 +
>  projects/libvirt-sandbox.yaml | 1 +
>  projects/libvirt-tck.yaml | 1 +
>  projects/libvirt.yaml | 1 +
>  projects/osinfo-db-tools.yaml | 1 +
>  projects/osinfo-db.yaml   | 1 +
>  projects/virt-manager.yaml| 1 +
>  projects/virt-viewer.yaml | 1 +
>  20 files changed, 20 insertions(+), 5 deletions(-)

Reviewed-by: Daniel P. Berrangé 


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

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

Re: [libvirt] [PATCH] qemu: monitor: Fix memory leak in qemuMonitorJSONNBDServerStart()

[Ján Tomko]

On Mon, Jun 25, 2018 at 07:41:54PM +0200, Michal Prívozník wrote:

On 06/25/2018 03:48 PM, w00251574 wrote:

Subject: [PATCH] qemu: monitor: Fix memory leak in 
qemuMonitorJSONNBDServerStart()

Exiting early through the return path did result in 'port_str'
being leaked.

Signed-off-by: Jie Wang 


Please fix your user.name in git config to match the signoff name:
https://git-scm.com/book/en/v2/Getting-Started-First-Time-Git-Setup

"w00251574" does not look like a human name.


---
 src/qemu/qemu_monitor_json.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


ACKed and pushed.



Please add 'checking the Author and Signoff' fields to your review
workflow.

Jano


Michal

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


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

Re: [libvirt] [PATCH 2/2] conf: Forbid device alias change on device-update

[Michal Privoznik]

On 06/15/2018 02:38 PM, John Ferlan wrote:
> 
> 
> On 06/12/2018 11:04 AM, Michal Privoznik wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1585108
>>
>> When updating a live device users might pass different alias than
>> the one the device has. Currently, this is silently ignored which
>> goes against our behaviour for other parts of the device where we
>> explicitly allow only certain changes and error out loudly on
>> anything else.
>>
>> Signed-off-by: Michal Privoznik 
>> ---
>>  src/conf/domain_conf.c | 12 +++-
>>  src/conf/domain_conf.h |  3 ++-
>>  src/lxc/lxc_driver.c   |  6 +++---
>>  src/qemu/qemu_driver.c | 16 
>>  4 files changed, 24 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 85f07af46e..91cac75c0a 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -28195,7 +28195,8 @@ virDomainDeviceInfoCheckBootIndex(virDomainDefPtr 
>> def ATTRIBUTE_UNUSED,
>>  int
>>  virDomainDefCompatibleDevice(virDomainDefPtr def,
>>   virDomainDeviceDefPtr dev,
>> - virDomainDeviceDefPtr oldDev)
>> + virDomainDeviceDefPtr oldDev,
>> + bool live)
>>  {
>>  virDomainCompatibleDeviceData data = {
>>  .newInfo = virDomainDeviceGetInfo(dev),
>> @@ -28205,6 +28206,15 @@ virDomainDefCompatibleDevice(virDomainDefPtr def,
>>  if (oldDev)
>>  data.oldInfo = virDomainDeviceGetInfo(oldDev);
>>  
>> +if (live &&
>> +((!!data.newInfo != !!data.oldInfo) ||
>> + (data.newInfo && data.oldInfo &&
>> +  STRNEQ(data.newInfo->alias, data.oldInfo->alias {
>> +virReportError(VIR_ERR_OPERATION_DENIED, "%s",
>> +   _("changing device alias is not allowed"));
>> +return -1;
>> +}
>> +
> 
> Does this work for attach-device from the bz?  I just tried it and it
> doesn't - probably because oldInfo would be NULL for attach.

Huh, okay. I guess I need to rename the argument to say "checkAlias" and
pass false from everywhere but qemuDomainUpdateDeviceLive(). Would you
agree with this solution?

> 
> Additionally, if the update/detach doesn't include an alias, then would
> this fail too?

That's the idea. In general, one can argue that when detaching a device,
only partial device XML could be parsed (the minimum needed to uniquely
identify the device). But with update device the semantic is changed.
Not only we use the XML to find the device we also use it to perform
changes: what's new is set, what's left out is unset. For instance,
imagine an  with QoS set. Then, call to update device with
the very same  with QoS left out must be treated as "user
wants QoS to be removed". Or vice versa. Because of this reason, leaving
out  is viewed as "unset alias" (which of course is not possible
for live XMLs, but is possible for inactive XMLs). Similarly, providing
different  is viewed as "change alias".

> 
> By comparison qemuDomainDiskChangeSupported only cares that the updated
> disk definition has an alias before making check and of course that
> would now be duplicitous to this.
> 
> Then there's qemuDomainChangeNet which at first I didn't understand why
> it wasn't throwing up since it does check the alias. Then I noticed
> something from the case, for the updated interface XML - if you include
> the  from the live guest, but change the "", then you
> get the error. However, if you remove the  from the update XML
> things then succeed. That's because of this gem in the code:
> 
> if (!virDomainDeviceAddressIsValid(>info,
> 
> VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) &&
> virDomainDeviceInfoCopy(>info, >info) < 0) {
> goto cleanup;
> }
> 
> where virDomainDeviceInfoCopy essentially *overwrites* the
> provided/changed newdev->info.alias.
> 
> So I think if you fix this part of the code, then you don't have the
> need for the rest of this patch. Yes, that does possibly leave the
> graphics devices as not checking alias updates, but that would seemingly
> be fixable separately if it's considered a problem or not checked as I
> didn't follow all the graphics update code.

Yep, this definitely deserves fixing (actually, the lines should be
removed, not providing  should be viewed as request for its
removal which is of course impossible). But I still think we need one
place where we would check for the alias change.

Michal

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

[libvirt] [jenkins-ci PATCH v2] jobs: Change git URL to fast external mirrors

[Pavel Hrdina]

We had our own local copy of all projects synchronized by cron on
the host where we have the CI VMs.  This was to save the traffic from
libvirt.org repositories and to make the cloning for our Jenkins jobs
faster.

We might move our VMs into a cloud in future we would not be able to
have any local copy so this changes the git URL to use fast external
mirrors.

Signed-off-by: Pavel Hrdina 
---

v1: 

 jobs/autotools.yaml   | 2 +-
 jobs/generic.yaml | 2 +-
 jobs/go.yaml  | 2 +-
 jobs/perl-modulebuild.yaml| 2 +-
 jobs/python-distutils.yaml| 2 +-
 projects/libosinfo.yaml   | 1 +
 projects/libvirt-cim.yaml | 1 +
 projects/libvirt-dbus.yaml| 1 +
 projects/libvirt-glib.yaml| 1 +
 projects/libvirt-go-xml.yaml  | 1 +
 projects/libvirt-go.yaml  | 1 +
 projects/libvirt-perl.yaml| 1 +
 projects/libvirt-python.yaml  | 1 +
 projects/libvirt-sandbox.yaml | 1 +
 projects/libvirt-tck.yaml | 1 +
 projects/libvirt.yaml | 1 +
 projects/osinfo-db-tools.yaml | 1 +
 projects/osinfo-db.yaml   | 1 +
 projects/virt-manager.yaml| 1 +
 projects/virt-viewer.yaml | 1 +
 20 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/jobs/autotools.yaml b/jobs/autotools.yaml
index f526ed2..c1d0f27 100644
--- a/jobs/autotools.yaml
+++ b/jobs/autotools.yaml
@@ -21,7 +21,7 @@
   num-to-keep: 1000
 scm:
   - git:
-  url: git://n64.pufty.ci.centos.org/{name}.git
+  url: '{git-url}'
   branches:
 - origin/{branch}
   clean:
diff --git a/jobs/generic.yaml b/jobs/generic.yaml
index 3e20962..6c59c63 100644
--- a/jobs/generic.yaml
+++ b/jobs/generic.yaml
@@ -21,7 +21,7 @@
   num-to-keep: 1000
 scm:
   - git:
-  url: git://n64.pufty.ci.centos.org/{name}.git
+  url: '{git-url}'
   branches:
 - origin/{branch}
   clean:
diff --git a/jobs/go.yaml b/jobs/go.yaml
index bffe56e..10518c4 100644
--- a/jobs/go.yaml
+++ b/jobs/go.yaml
@@ -21,7 +21,7 @@
   num-to-keep: 1000
 scm:
   - git:
-  url: git://n64.pufty.ci.centos.org/{name}.git
+  url: '{git-url}'
   branches:
 - origin/{branch}
   clean:
diff --git a/jobs/perl-modulebuild.yaml b/jobs/perl-modulebuild.yaml
index 4a79bab..934b216 100644
--- a/jobs/perl-modulebuild.yaml
+++ b/jobs/perl-modulebuild.yaml
@@ -21,7 +21,7 @@
   num-to-keep: 1000
 scm:
   - git:
-  url: git://n64.pufty.ci.centos.org/{name}.git
+  url: '{git-url}'
   branches:
 - origin/{branch}
   clean:
diff --git a/jobs/python-distutils.yaml b/jobs/python-distutils.yaml
index c075245..0b20b17 100644
--- a/jobs/python-distutils.yaml
+++ b/jobs/python-distutils.yaml
@@ -21,7 +21,7 @@
   num-to-keep: 1000
 scm:
   - git:
-  url: git://n64.pufty.ci.centos.org/{name}.git
+  url: '{git-url}'
   branches:
 - origin/{branch}
   clean:
diff --git a/projects/libosinfo.yaml b/projects/libosinfo.yaml
index 8e3d105..22c957e 100644
--- a/projects/libosinfo.yaml
+++ b/projects/libosinfo.yaml
@@ -3,6 +3,7 @@
 name: libosinfo
 machines: '{all_machines}'
 title: libosinfo
+git-url: https://gitlab.com/libosinfo/libosinfo.git
 jobs:
   - autotools-build-job:
   parent_jobs: 'osinfo-db-master-build'
diff --git a/projects/libvirt-cim.yaml b/projects/libvirt-cim.yaml
index dff3976..c6a7a6d 100644
--- a/projects/libvirt-cim.yaml
+++ b/projects/libvirt-cim.yaml
@@ -3,6 +3,7 @@
 name: libvirt-cim
 machines: '{rpm_machines}'
 title: libvirt CIM
+git-url: https://github.com/libvirt/libvirt-cim.git
 jobs:
   - autotools-build-job:
   parent_jobs: 'libvirt-master-build'
diff --git a/projects/libvirt-dbus.yaml b/projects/libvirt-dbus.yaml
index c460db4..44d1b23 100644
--- a/projects/libvirt-dbus.yaml
+++ b/projects/libvirt-dbus.yaml
@@ -2,6 +2,7 @@
 - project:
 name: libvirt-dbus
 title: Libvirt D-Bus
+git-url: https://github.com/libvirt/libvirt-dbus.git
 jobs:
   - autotools-build-job:
   parent_jobs: 'libvirt-glib-master-build'
diff --git a/projects/libvirt-glib.yaml b/projects/libvirt-glib.yaml
index 286d25b..f61ca1e 100644
--- a/projects/libvirt-glib.yaml
+++ b/projects/libvirt-glib.yaml
@@ -3,6 +3,7 @@
 name: libvirt-glib
 machines: '{all_machines}'
 title: Libvirt GLib
+git-url: https://github.com/libvirt/libvirt-glib.git
 jobs:
   - autotools-build-job:
   parent_jobs: 'libvirt-master-build'
diff --git a/projects/libvirt-go-xml.yaml b/projects/libvirt-go-xml.yaml
index 8d9c16d..6eb7ef0 100644
--- a/projects/libvirt-go-xml.yaml
+++ b/projects/libvirt-go-xml.yaml
@@ -3,6 +3,7 @@
 name: libvirt-go-xml
 machines: '{all_machines}'
 title: Libvirt Go XML
+git-url: 

Re: [libvirt] [PATCH 3/7] conf: Reintroduce virDomainDef::hpt_resizing

[Michal Privoznik]

On 06/25/2018 07:11 PM, Andrea Bolognani wrote:
> We're going to introduce a second HPT-related setting soon,
> at which point using a single location to store everything is
> no longer going to cut it.
> 
> This mostly, but not completely, reverts 3dd1eb3b2650.
> 
> Signed-off-by: Andrea Bolognani 
> ---
>  src/conf/domain_conf.c  | 21 ++---
>  src/conf/domain_conf.h  |  1 +
>  src/qemu/qemu_command.c |  7 ---
>  src/qemu/qemu_domain.c  |  2 +-
>  4 files changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index f0b604e125..069ff8c2f8 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -19803,9 +19803,12 @@ virDomainDefParseXML(xmlDocPtr xml,
> tmp);
>  goto error;
>  }
> -def->features[val] = value;
> +def->hpt_resizing = (virDomainHPTResizing) value;

This typecast seems useless. Also, pre-existing, the if() statement
above is more verbose than it needs to be since
VIR_DOMAIN_HPT_RESIZING_NONE is defined to be zero.

>  VIR_FREE(tmp);
>  }
> +
> +if (def->hpt_resizing != VIR_DOMAIN_HPT_RESIZING_NONE)
> +def->features[val] = VIR_TRISTATE_SWITCH_ON;
>  break;
>  
>  /* coverity[dead_error_begin] */


Michal

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

Re: [libvirt] [PATCH 0/7] qemu: Support page size tuning for pSeries guests

[Michal Privoznik]

On 06/25/2018 07:11 PM, Andrea Bolognani wrote:
> This applies cleanly on top of a0d6894af1b1.
> 
> Patch 1/7 is too big to go through the list; it can be fetched,
> along with the rest of the series, from [1].
> 
> Patch 2/7 conflicts with patch 1/3 from [2], but making this one
> depend on it didn't feel right; whichever of the two series gets
> reviewed and merged first, I'll probably post a rebase of the
> other one immediately afterwards.
> 
> Changes from [RFC]:
> 
> * the QEMU interface has changed based on feedback, which means
>   some of the code is no longer useful and has been dropped;
> * switched to virXMLFormatElement(), as suggested during review;
> * added documentation and release notes entry.
> 
> 
> [1] https://github.com/andreabolognani/libvirt/tree/pseries-hpt-maxpagesize

would be nice if the branch would be derived from the master you have
there too ;-)

> [2] https://www.redhat.com/archives/libvir-list/2018-June/msg01374.html
> [RFC] https://www.redhat.com/archives/libvir-list/2018-May/msg01710.html
> 
> Andrea Bolognani (7):
>   tests: Add replies for QEMU 3.0.0 on ppc64
>   qemu: Add capability for the HPT maxpagesize feature
>   conf: Reintroduce virDomainDef::hpt_resizing
>   conf: Tweak HPT feature parsing and formatting
>   conf: Parse and format HPT maxpagesize
>   qemu: Format HPT maxpagesize on the command line
>   news: Update for HPT maxpagesize feature
> 
>  docs/formatdomain.html.in |   11 +-
>  docs/news.xml |   11 +
>  docs/schemas/domaincommon.rng |   21 +-
>  src/conf/domain_conf.c|   60 +-
>  src/conf/domain_conf.h|2 +
>  src/qemu/qemu_capabilities.c  |8 +
>  src/qemu/qemu_capabilities.h  |1 +
>  src/qemu/qemu_command.c   |   43 +-
>  src/qemu/qemu_domain.c|2 +-
>  .../caps_2.12.0.aarch64.replies   |   48 +-
>  .../caps_2.12.0.aarch64.xml   |2 +-
>  .../caps_2.12.0.ppc64.replies |  197 +-
>  .../caps_2.12.0.ppc64.xml |2 +-
>  .../caps_2.12.0.s390x.replies |   52 +-
>  .../caps_2.12.0.s390x.xml |2 +-
>  .../caps_2.12.0.x86_64.replies|   64 +-
>  .../caps_2.12.0.x86_64.xml|2 +-
>  ...ppc64.replies => caps_3.0.0.ppc64.replies} | 5268 ++---
>  ..._2.12.0.ppc64.xml => caps_3.0.0.ppc64.xml} |   19 +-
>  tests/qemucapabilitiestest.c  |1 +
>  tests/qemuxml2argvdata/pseries-features.args  |3 +-
>  tests/qemuxml2argvdata/pseries-features.xml   |   18 +-
>  tests/qemuxml2argvtest.c  |1 +
>  tests/qemuxml2xmloutdata/pseries-features.xml |   31 +-
>  tests/qemuxml2xmltest.c   |1 +
>  25 files changed, 3624 insertions(+), 2246 deletions(-)
>  copy tests/qemucapabilitiesdata/{caps_2.12.0.ppc64.replies => 
> caps_3.0.0.ppc64.replies} (92%)
>  copy tests/qemucapabilitiesdata/{caps_2.12.0.ppc64.xml => 
> caps_3.0.0.ppc64.xml} (99%)
>  mode change 12 => 100644 tests/qemuxml2xmloutdata/pseries-features.xml
> 

ACK

Michal

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

[libvirt] Plan for next releases

[Daniel Veillard]

  The end of the month is coming quickly, and if we want to release on time
it seems we should enter freeze tomorrow wed 27. Then I can make RC2 on Friday
and push next Monday assuming thereis no blocker.

  Hope this works for everybody,

   thanks,

Daniel

-- 
Daniel Veillard  | Red Hat Developers Tools http://developer.redhat.com/
veill...@redhat.com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | virtualization library  http://libvirt.org/

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

Re: [libvirt] [GSoC] Code design for scalar and external types

[Pavel Hrdina]

On Mon, Jun 25, 2018 at 10:22:14AM +0200, Erik Skultety wrote:
> On Mon, Jun 18, 2018 at 11:52:04AM +0100, Daniel P. Berrangé wrote:
> > On Mon, Jun 18, 2018 at 12:24:39PM +0200, Pavel Hrdina wrote:
> > > Hi,
> > >
> > > It took me longer to sit down and write this mail but here it goes.
> > >
> > > There was a lot of ideas about the macro design and it's easy to
> > > get lost in all the designs.
> > >
> > > So we agreed on this form:
> > >
> > >
> > > # define VIR_AUTOPTR_FUNC_NAME(type) virAutoPtr##type
> > > # define VIR_AUTOCLEAR_FUNC_NAME(type) virAutoClear##type
> > >
> > > # define VIR_DEFINE_AUTOPTR_FUNC(type, func) \
> > > static inline void VIR_AUTOPTR_FUNC_NAME(type)(type *_ptr) \
> > > { \
> > > (func)(*_ptr); \
> > > }
> > >
> > > # define VIR_DEFINE_AUTOCLEAR_FUNC(type, func) \
> > > static inline void VIR_AUTOCLEAR_FUNC_NAME(type)(type *_ptr) \
> > > { \
> > > (func)(_ptr); \
> > > }
> >
> > For anything where we define the impl of (func) these two macros
> > should never be used IMHO. We should just change the signature of
> > the real functions so that accept a double pointer straight away,
> > and thus not require the wrapper function. Yes, it this will
> > require updating existing code, but such a design change is
> > beneficial even without the cleanup macros, as it prevents the
> > possbility of double frees. I'd suggest we just do this kind of
> > change straightaway
> 
> >From security POV, I agree that the free functions should accept a double
> pointer. However, in terms of GSoC and the time schedule, I think that having
> the macros above is a nice stepping stone towards to long term goal to convert
> our free functions to accept double pointers, besides, as you pointed out
> below, the macros (at least the AUTOPTR_FUNC) makes sense with external types
> and I like that we'd have a almost universal approach here. Even with the
> macros, we don't lose anything (because they're straightforward and that's
> important), quite the opposite, it's progress compared to the current status
> quo.
> 
> >
> > > # define VIR_AUTOFREE(type) __attribute__((cleanup(virFree))) type
> >
> > I don't see the point in passing "type" in here we could simplify
> > this:
> >
> >   #define VIR_AUTOFREE __attribute__((cleanup(virFree))
> >
> >   VIR_AUTOFREE char *foo = NULL;
> >
> > and at the same time fix the char ** problems
> >
> >   #define VIR_AUTOFREE_FUNC(func) __attribute__((cleanup(func))
> >
> >   VIR_AUTOFREE_FUNC(virStringListFree) char *foo = NULL;
> >
> > or if we want to simplify it
> >
> >   #define VIR_AUTOFREE_STRINGLIST VIR_AUTOFREE_FUNC(virStringListFree)
> >
> >   VIR_AUTOFREE_STRINGLIST  char **strs = NULL;
> 
> Andrea already pointed it out in his reply, but I don't like this
> "simplification" either, the syntax is kinda obfuscating.
> 
> >
> >
> > This also works for external libraries
> >
> >
> >
> > > # define VIR_AUTOPTR(type) \
> > > __attribute__((cleanup(VIR_AUTOPTR_FUNC_NAME(type type
> > >
> > > # define VIR_AUTOCLEAR(type) \
> > > __attribute__((cleanup(VIR_AUTOCLEAR_FUNC_NAME(type type
> > >
> > >
> > > but it turned out the it's not sufficient for several reasons:
> > >
> > > - there is no simple way ho to free list of strings (char **)
> > > - this will not work for external types with their own free function
> > >
> > >
> > > In order to solve these two issues there were some ideas.
> > >
> > >
> > > 1. How to handle list of strings
> > >
> > > We have virStringListFree() function in order to free list of strings,
> > > the question is how to use it with these macros:
> > >
> > > - Create a new typedef for list of strings and use VIR_AUTOPTR:
> > >
> > > typedef char **virStringList;
> > >
> > > VIR_DEFINE_AUTOPTR_FUNC(virStringList, virStringListFree);
> > >
> > > VIR_AUTOPTR(virStringList) list = NULL;
> >
> > I don't think we should be creating artifical new typedefs just to
> > deal with the flawed design of our own autofree macros.
> >
> > > - Overload VIR_AUTOFREE macro by making it variadic
> > >
> > > # define _VIR_AUTOFREE_0(type) __attribute__((cleanup(virFree))) type
> > > # define _VIR_AUTOFREE_1(type, func) __attribute__((cleanup(func))) type
> > >
> > > # define _VIR_AUTOFREE_OVERLOADER(_1, _2, NAME, ...) NAME
> > > # define VIR_AUTOFREE(...) _VIR_AUTOFREE_OVERLOADER(__VA_ARGS__, 
> > > _VIR_AUTOFREE_1, _VIR_AUTOFREE_0)(__VA_ARGS__)
> >
> > This is uneccessarily black magic - just have VIR_AUTOFREE and
> > VIR_AUTOFREE_FUNC defined separately.
> 
> Yeah, looking back at when I came up with the idea, it is indeed unnecessarily
> hacky. Having another macro for this purpose is an option, although Pavel had
> some concerns that if we do that, people might misuse it every time they don't
> know what the proper usage or approach is in that the type name and the
> corresponding free function name should match. To me, that sounds like a
> reviewer's job to spot. On the other hand, I 

Re: [libvirt] [PATCH] util: fix mount issue by moving NULL value to "none" in syscall.

[Michal Privoznik]

On 06/26/2018 05:18 AM, Julio Faracco wrote:
> After running libvirt daemon with valgrind tools, some errors are
> appearing when you try to start a domain. One example:
> 
> ==18012== Syscall param mount(type) points to unaddressable byte(s)
> ==18012==at 0x6FEE3CA: mount (syscall-template.S:78)
> ==18012==by 0x531344D: virFileMoveMount (virfile.c:3828)
> ==18012==by 0x27FE7675: qemuDomainBuildNamespace (qemu_domain.c:11501)
> ==18012==by 0x2800C44E: qemuProcessHook (qemu_process.c:2870)
> ==18012==by 0x52F7E1D: virExec (vircommand.c:726)
> ==18012==by 0x52F7E1D: virCommandRunAsync (vircommand.c:2477)
> ==18012==by 0x52F4EDD: virCommandRun (vircommand.c:2309)
> ==18012==by 0x2800A731: qemuProcessLaunch (qemu_process.c:6235)
> ==18012==by 0x2800D6B4: qemuProcessStart (qemu_process.c:6569)
> ==18012==by 0x28074876: qemuDomainObjStart (qemu_driver.c:7314)
> ==18012==by 0x280522EB: qemuDomainCreateWithFlags (qemu_driver.c:7367)
> ==18012==by 0x55484BF: virDomainCreate (libvirt-domain.c:6531)
> ==18012==by 0x12CDBD: remoteDispatchDomainCreate 
> (remote_daemon_dispatch_stubs.h:4350)
> ==18012==by 0x12CDBD: remoteDispatchDomainCreateHelper 
> (remote_daemon_dispatch_stubs.h:4326)
> ==18012==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
> 
> Some documentation recommends to use "none" when you don't have a
> filesystem type to use. Specially, for bind and move actions.
> 
> Signed-off-by: Julio Faracco 
> ---
>  src/util/virfile.c| 2 +-
>  src/util/virprocess.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

ACKed and pushed. Although, there are some more similar calls, in case
you want to fix it.

Michal

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