Re: [libvirt] [PATCH 11/17] util: Introduce libvirt_udevhelper

[Jiri Denemark]

On Wed, Oct 26, 2016 at 17:39:35 +0200, Daniel P. Berrange wrote:
> On Wed, Oct 26, 2016 at 02:36:58PM +0200, Michal Privoznik wrote:
> > This is a small helper intended to be run by udev. On its input
> > (either as the only command line argument or in DEVNODE
> > environment vairable) it is given a device and on the output it
> > will either put nothing (meaning the device is not used by any of
> > the libvirt domains), or it will print out security labels in the
> > following form:
> > 
> >   UID GID SELABEL
> 
> How is this intended to be actually used ? ie what udev rule are
> you creating along with this ?

Yeah, the rule should really be part of this series.

> IMHO we just want the helper to indicate that udev should not do
> anything to the device - we should not need udev to ever set labels
> itself as libvirt has already set them - we just don't want udev to
> remove them. IOW, I don't see the need to print out this info at all.

That would be nice, but unfortunately there's no way to tell udev not to
touch a specific device (I discussed this stuff with Michal Sekletar).
Other udev rules might have already set UID/GID/SELABEL for the device
and we can only change it to contain the required content; we can't
reset them to "don't change any of these".

And if you were thinking that our rule could be the first rule called on
each device (rather than the last one), there's no way to tell udev to
just skip all other rules and ignore the device. It will run through all
rules and they were set their own UID/GID/SELABEL as they wish.

Jirka

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

Re: [libvirt] [PATCH 1/1] qemu: host NUMA hugepage policy without guest NUMA

[Sam Bobroff]

On Tue, Oct 25, 2016 at 02:13:07PM +0200, Martin Kletzander wrote:
> On Tue, Oct 25, 2016 at 01:10:23PM +1100, Sam Bobroff wrote:
> >On Tue, Oct 18, 2016 at 10:43:31PM +0200, Martin Kletzander wrote:
> >>On Mon, Oct 17, 2016 at 03:45:09PM +1100, Sam Bobroff wrote:
> >>>On Fri, Oct 14, 2016 at 10:19:42AM +0200, Martin Kletzander wrote:
> On Fri, Oct 14, 2016 at 11:52:22AM +1100, Sam Bobroff wrote:
> >I did look at the libnuma and cgroups approaches, but I was concerned 
> >they
> >wouldn't work in this case, because of the way QEMU allocates memory when
> >mem-prealloc is used: the memory is allocated in the main process, 
> >before the
> >CPU threads are created. (This is based only on a bit of hacking and 
> >debugging
> >in QEMU, but it does seem explain the behaviour I've seen so far.)
> >
> 
> But we use numactl before QEMU is exec()'d.
> >>>
> >>>Sorry, I jumped ahead a bit. I'll try to explain what I mean:
> >>>
> >>>I think the problem with using this method would be that the NUMA policy is
> >>>applied to all allocations by QEMU, not just ones related to the memory
> >>>backing. I'm not sure if that would cause a serious problem but it seems 
> >>>untidy,
> >>>and it doesn't happen in other situations (i.e. with separate memory 
> >>>backend
> >>>objects, QEMU sets up the policy specifically for each one and other
> >>>allocations aren't affected, AFAIK).  Presumably, if memory were very
> >>>restricted it could prevent the guest from starting.
> >>>
> >>
> >>Yes, it is, that's what  does if you don't have any
> >>other () specifics set.
> >>
> >I think QEMU could be altered to move the preallocations into the VCPU
> >threads but it didn't seem trivial and I suspected the QEMU community 
> >would
> >point out that there was already a way to do it using backend objects.  
> >Another
> >option would be to add a -host-nodes parameter to QEMU so that the 
> >policy can
> >be given without adding a memory backend object. (That seems like a more
> >reasonable change to QEMU.)
> >
> 
> I think upstream won't like that, mostly because there is already a
> way.  And that is using memory-backend object.  I think we could just
> use that and disable changing it live.  But upstream will probably want
> that to be configurable or something.
> >>>
> >>>Right, but isn't this already an issue in the cases where libvirt is 
> >>>already
> >>>using memory backend objects and NUMA policy? (Or does libvirt already 
> >>>disable
> >>>changing it live in those situations?)
> >>>
> >>
> >>It is.  I'm not trying to say libvirt is perfect.  There are bugs,
> >>e.g. like this one.  The problem is that we tried to do *everything*,
> >>but it's not currently possible.  I'm trying to explain how stuff works
> >>now.  It definitely needs some fixing, though.
> >
> >OK :-)
> >
> >Well, given our discussion, do you think it's worth a v2 of my original patch
> >or would it be better to drop it in favour of some broader change?
> >
> 
> Honestly, I thought about the approaches so much I'm now not sure I'll
> make a good decision.  RFC could do.  If I were to pick, I would go with
> a new setting that would control whether we want the binding to be
> changeable throughout the domain's lifetime or not so that we can make
> better decisions (and don't feel bad about the bad ones).

I feel the same way.

OK, I'll try an RFC patch with a lot of description.

I'm specifically trying to address the issue I originally raised, which isn't
quite the same thing as the changeability of the bindings but I'll keep that in
mind. I think your point about changing the bindings will apply in the
same way whenever QEMU's memory-backend objects are used with their
"host-nodes" attribute (since they are what causes QEMU to apply policy), so I
don't think I'm suggesting any significant change there.

If you want to add the new setting you mention above, I'd be happy to base my
patch of top of that work. ;-)

Cheers,
Sam.

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

[libvirt] [PATCH v3 0/9] Implementation of QEMU vhost-scsi

[Eric Farman]

[Author note: Apologies for an extra release or two in between versions,
I was sidetracked by another project. This is probably too close to the
impending freeze for 2.4, so I just updated doc to 2.5 in anticipation.
In rearranging the patches, I've inserted a cleanup patch at the head
that was mentioned in the v2 review, and which could go separately.]

This patch series provides a libvirt implementation of the vhost-scsi
interface in QEMU.  As near as I can see, this was discussed upstream in
July 2014[1], and ended in a desire to replace a vhost-scsi controller
in favor of a hostdev element instead[2].

Host Filesystem Example:
  # ls /sys/kernel/config/target/vhost/
  discovery_auth  naa.5001405df3e54061  version
  # ls /sys/kernel/config/target/vhost/naa.5001405df3e54061/tpgt_1/lun/
  lun_0

QEMU Example (snippet):
  -device vhost-scsi-ccw,wwpn=naa.5001405df3e54061,devno=fe.0.1000

Libvirt Example (snippet):
  


  

Guest Viewpoint:
  # lsscsi
  [1:0:1:0]diskLIO-ORG  disk04.0   /dev/sda 
  # dmesg | grep 1:
  [6.065735] scsi host1: Virtio SCSI HBA
  [6.093892] scsi 1:0:1:0: Direct-Access LIO-ORG  disk04.0  
PQ: 0 ANSI: 5
  [6.313615] sd 1:0:1:0: Attached scsi generic sg0 type 0
  [6.314981] sd 1:0:1:0: [sda] 29360128 512-byte logical blocks: (15.0 
GB/14.0 GiB)
  [6.317290] sd 1:0:1:0: [sda] Write Protect is off
  [6.317566] sd 1:0:1:0: [sda] Mode Sense: 43 00 10 08
  [6.317853] sd 1:0:1:0: [sda] Write cache: enabled, read cache: enabled, 
supports DPO and FUA
  [6.352722] sd 1:0:1:0: [sda] Attached SCSI disk

Changelog:

  v2.1->v3:
   - Rebase
  - Rebased to current master (26 October)
   - Comments
  - Added an early patch to do some additional typecasting in the
switch statements of hostdev.subsys.type
  - Did some reordering of patches, to hopefully flow better
  - Implemented an activeHostHostdevs list, which is used by the
cgroup and security codepaths
  - doc changes -- s/2.2/2.5/ and s/HBI/HBA/
  - Added a "none" protocol type for scsi_host hostdevs (which is invalid)
  - Restored the apparmor and selinux codepaths that got lost from v1
  - Added a proper check for a valid scsi_host protocol, and saving that
value within the HostdevDef struct
  - Fixed a compiler warning with call to virDomainPCIAddressEnsureAddr
  - Removed the rest of vhostfdSize, since multiple fd's are not allowed
by QEMU
  - Fixed cleanup of vhostfd in error from building -device string
  - Moved the "conf" chunk from "hotplug" patch to "introduce" patch
  - Added xml2xml test
  - Added a proper calculation of "address" in virDomainAuditHostdev
  - Added a virFileExists check before open(/dev/vhost-scsi)
  - Addressed a number of lines >80 characters
   - Things *NOT* done (later?)
  - Investigation/tie-in with virsh nodedev-list stuff
  - Implementation of 'num_queues', 'max_sectors', and 'cmd_per_lun'
(Need to research these in the virtio space, before figuring out
how to apply to vhost-scsi)
  - Dropping the "naa." prefix of wwn
  - Split the "tests" patch into earlier patches
   - Other

  v2.1:  
https://www.redhat.com/archives/libvir-list/2016-September/msg00148.html
  v2:https://www.redhat.com/archives/libvir-list/2016-August/msg01028.html
  v1:https://www.redhat.com/archives/libvir-list/2016-July/msg01004.html

[1] http://www.redhat.com/archives/libvir-list/2014-July/msg01235.html
[2] http://www.redhat.com/archives/libvir-list/2014-July/msg01390.html

Eric Farman (9):
  qemu: Introduce vhost-scsi capability
  Introduce a "scsi_host" hostdev type
  util: Management routines for scsi_host devices
  qemu: Add vhost-scsi string for -device parameter
  qemu: Allow hotplug of vhost-scsi device
  conf: Wire up the vhost-scsi connection from/to XML
  security: Include vhost-scsi in security labels
  tests: Introduce basic vhost-scsi test
  docs: Add vhost-scsi

 docs/formatdomain.html.in  |  24 ++
 docs/schemas/domaincommon.rng  |  23 ++
 src/Makefile.am|   1 +
 src/conf/domain_audit.c|   7 +
 src/conf/domain_conf.c |  91 ++-
 src/conf/domain_conf.h |  18 ++
 src/libvirt_private.syms   |  19 ++
 src/qemu/qemu_capabilities.c   |   2 +
 src/qemu/qemu_capabilities.h   |   1 +
 src/qemu/qemu_cgroup.c |  39 +++
 src/qemu/qemu_command.c|  79 ++
 src/qemu/qemu_command.h|   5 +
 src/qemu/qemu_domain_address.c |  10 +
 src/qemu/qemu_hostdev.c|  41 +++
 src/qemu/qemu_hostdev.h|   8 +
 src/qemu/qemu_hotplug.c  

[libvirt] [PATCH v3 7/9] security: Include vhost-scsi in security labels

[Eric Farman]

Signed-off-by: Eric Farman 
---
 src/security/security_apparmor.c | 18 -
 src/security/security_dac.c  | 42 ++--
 src/security/security_selinux.c  | 39 +++--
 3 files changed, 94 insertions(+), 5 deletions(-)

diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
index e7e3c8c..d7bc7d1 100644
--- a/src/security/security_apparmor.c
+++ b/src/security/security_apparmor.c
@@ -44,6 +44,7 @@
 #include "viruuid.h"
 #include "virpci.h"
 #include "virusb.h"
+#include "virhost.h"
 #include "virfile.h"
 #include "configmake.h"
 #include "vircommand.h"
@@ -357,6 +358,13 @@ AppArmorSetSecuritySCSILabel(virSCSIDevicePtr dev 
ATTRIBUTE_UNUSED,
 return AppArmorSetSecurityHostdevLabelHelper(file, opaque);
 }
 
+static int
+AppArmorSetSecurityHostLabel(virHostDevicePtr dev ATTRIBUTE_UNUSED,
+ const char *file, void *opaque)
+{
+return AppArmorSetSecurityHostdevLabelHelper(file, opaque);
+}
+
 /* Called on libvirtd startup to see if AppArmor is available */
 static int
 AppArmorSecurityManagerProbe(const char *virtDriver ATTRIBUTE_UNUSED)
@@ -831,6 +839,7 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr,
 virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb;
 virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci;
 virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi;
+virDomainHostdevSubsysHostPtr hostsrc = &dev->source.subsys.u.host;
 
 if (!secdef)
 return -1;
@@ -910,7 +919,14 @@ AppArmorSetSecurityHostdevLabel(virSecurityManagerPtr mgr,
 }
 
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST: {
-/* Fall through for now */
+virHostDevicePtr host = virHostDeviceNew(hostsrc->wwpn);
+
+if (!host)
+goto done;
+
+ret = virHostDeviceFileIterate(host, AppArmorSetSecurityHostLabel, 
ptr);
+virHostDeviceFree(host);
+break;
 }
 
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index eba2a87..accb965 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -36,6 +36,7 @@
 #include "virpci.h"
 #include "virusb.h"
 #include "virscsi.h"
+#include "virhost.h"
 #include "virstoragefile.h"
 #include "virstring.h"
 #include "virutil.h"
@@ -582,6 +583,15 @@ virSecurityDACSetSCSILabel(virSCSIDevicePtr dev 
ATTRIBUTE_UNUSED,
 
 
 static int
+virSecurityDACSetHostLabel(virHostDevicePtr dev ATTRIBUTE_UNUSED,
+   const char *file,
+   void *opaque)
+{
+return virSecurityDACSetHostdevLabelHelper(file, opaque);
+}
+
+
+static int
 virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr,
   virDomainDefPtr def,
   virDomainHostdevDefPtr dev,
@@ -592,6 +602,7 @@ virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr,
 virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb;
 virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci;
 virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi;
+virDomainHostdevSubsysHostPtr hostsrc = &dev->source.subsys.u.host;
 int ret = -1;
 
 if (!priv->dynamicOwnership)
@@ -677,7 +688,14 @@ virSecurityDACSetHostdevLabel(virSecurityManagerPtr mgr,
 }
 
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST: {
-/* Fall through for now */
+virHostDevicePtr host = virHostDeviceNew(hostsrc->wwpn);
+
+if (!host)
+goto done;
+
+ret = virHostDeviceFileIterate(host, virSecurityDACSetHostLabel, 
&cbdata);
+virHostDeviceFree(host);
+break;
 }
 
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
@@ -724,6 +742,17 @@ virSecurityDACRestoreSCSILabel(virSCSIDevicePtr dev 
ATTRIBUTE_UNUSED,
 
 
 static int
+virSecurityDACRestoreHostLabel(virHostDevicePtr dev ATTRIBUTE_UNUSED,
+   const char *file,
+   void *opaque)
+{
+virSecurityManagerPtr mgr = opaque;
+virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+return virSecurityDACRestoreFileLabel(priv, file);
+}
+
+
+static int
 virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
   virDomainDefPtr def,
   virDomainHostdevDefPtr dev,
@@ -735,6 +764,7 @@ virSecurityDACRestoreHostdevLabel(virSecurityManagerPtr mgr,
 virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb;
 virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci;
 virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi;
+virDomainHostdevSubsysHostPtr hostsrc = &dev->source.subsys.u.host;
 int ret = -1;
 
 secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
@@ -810,7 +840,15 @@ virSecurityDACRestoreHostdevLabel(virSecurity

[libvirt] [PATCH v3 4/9] qemu: Add vhost-scsi string for -device parameter

[Eric Farman]

Open /dev/vhost-scsi, and record the resulting file descriptor, so that
the guest has access to the host device outside of the libvirt daemon.
Pass this information, along with data parsed from the XML file, to build
a device string for the qemu command line.  That device string will be
for either a vhost-scsi-ccw device in the case of an s390 machine, or
vhost-scsi-pci for any others.

Signed-off-by: Eric Farman 
---
 src/qemu/qemu_cgroup.c | 32 +
 src/qemu/qemu_command.c| 79 ++
 src/qemu/qemu_command.h|  5 +++
 src/qemu/qemu_domain_address.c | 10 ++
 src/qemu/qemu_hostdev.c| 41 ++
 src/qemu/qemu_hostdev.h|  8 +
 6 files changed, 175 insertions(+)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index ee31d14..a22a1bf 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -277,6 +277,25 @@ qemuSetupHostSCSIDeviceCgroup(virSCSIDevicePtr dev 
ATTRIBUTE_UNUSED,
 return ret;
 }
 
+static int
+qemuSetupHostHostDeviceCgroup(virHostDevicePtr dev ATTRIBUTE_UNUSED,
+  const char *path,
+  void *opaque)
+{
+virDomainObjPtr vm = opaque;
+qemuDomainObjPrivatePtr priv = vm->privateData;
+int ret;
+
+VIR_DEBUG("Process path '%s' for scsi_host device", path);
+
+ret = virCgroupAllowDevicePath(priv->cgroup, path,
+   VIR_CGROUP_DEVICE_RW, false);
+
+virDomainAuditCgroupPath(vm, priv->cgroup, "allow", path, "rw", ret == 0);
+
+return ret;
+}
+
 int
 qemuSetupHostdevCgroup(virDomainObjPtr vm,
virDomainHostdevDefPtr dev)
@@ -286,9 +305,11 @@ qemuSetupHostdevCgroup(virDomainObjPtr vm,
 virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb;
 virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci;
 virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi;
+virDomainHostdevSubsysHostPtr hostsrc = &dev->source.subsys.u.host;
 virPCIDevicePtr pci = NULL;
 virUSBDevicePtr usb = NULL;
 virSCSIDevicePtr scsi = NULL;
+virHostDevicePtr host = NULL;
 char *path = NULL;
 
 /* currently this only does something for PCI devices using vfio
@@ -377,6 +398,16 @@ qemuSetupHostdevCgroup(virDomainObjPtr vm,
 }
 
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST: {
+if (hostsrc->protocol ==
+VIR_DOMAIN_HOSTDEV_SUBSYS_HOST_PROTOCOL_TYPE_VHOST) {
+if ((host = virHostDeviceNew(hostsrc->wwpn)) == NULL)
+goto cleanup;
+
+if (virHostDeviceFileIterate(host,
+ qemuSetupHostHostDeviceCgroup,
+ vm) < 0)
+goto cleanup;
+}
 break;
 }
 
@@ -390,6 +421,7 @@ qemuSetupHostdevCgroup(virDomainObjPtr vm,
 virPCIDeviceFree(pci);
 virUSBDeviceFree(usb);
 virSCSIDeviceFree(scsi);
+virHostDeviceFree(host);
 VIR_FREE(path);
 return ret;
 }
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b68da3d..4e946ea 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4722,6 +4722,43 @@ qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr 
dev)
 }
 
 char *
+qemuBuildHostHostdevDevStr(const virDomainDef *def,
+   virDomainHostdevDefPtr dev,
+   virQEMUCapsPtr qemuCaps,
+   char *vhostfdName)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+virDomainHostdevSubsysHostPtr hostsrc = &dev->source.subsys.u.host;
+
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VHOST_SCSI)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("This QEMU doesn't support vhost-scsi devices"));
+goto cleanup;
+}
+
+if (ARCH_IS_S390(def->os.arch))
+virBufferAddLit(&buf, "vhost-scsi-ccw");
+else
+virBufferAddLit(&buf, "vhost-scsi-pci");
+
+virBufferAsprintf(&buf, ",wwpn=%s", hostsrc->wwpn);
+virBufferAsprintf(&buf, ",vhostfd=%s", vhostfdName);
+virBufferAsprintf(&buf, ",id=%s", dev->info->alias);
+
+if (qemuBuildDeviceAddressStr(&buf, def, dev->info, qemuCaps) < 0)
+goto cleanup;
+
+if (virBufferCheckError(&buf) < 0)
+goto cleanup;
+
+return virBufferContentAndReset(&buf);
+
+ cleanup:
+virBufferFreeAndReset(&buf);
+return NULL;
+}
+
+char *
 qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev)
 {
 virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -5202,6 +5239,48 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd,
 return -1;
 }
 }
+
+/* SCSI_host */
+if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
+subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS

[libvirt] [PATCH v3 5/9] qemu: Allow hotplug of vhost-scsi device

[Eric Farman]

Adjust the device string that is built for vhost-scsi devices so that it
can be invoked from hotplug.

>From the QEMU command line, the file descriptors are expect to be numeric only.
However, for hotplug, the file descriptors are expected to begin with at least
one alphabetic character else this error occurs:

  # virsh attach-device guest_0001 ~/vhost.xml
  error: Failed to attach device from /root/vhost.xml
  error: internal error: unable to execute QEMU command 'getfd':
  Parameter 'fdname' expects a name not starting with a digit

We also close the file descriptor in this case, so that shutting down the
guest cleans up the host cgroup entries and allows future guests to use
vhost-scsi devices.  (Otherwise the guest will silently end.)

Signed-off-by: Eric Farman 
---
 src/qemu/qemu_hotplug.c | 158 
 1 file changed, 158 insertions(+)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 335252b..33e55e3 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2434,6 +2434,120 @@ qemuDomainAttachHostSCSIDevice(virConnectPtr conn,
 goto cleanup;
 }
 
+static int
+qemuDomainAttachHostSCSIHostDevice(virQEMUDriverPtr driver,
+   virDomainObjPtr vm,
+   virDomainHostdevDefPtr hostdev)
+{
+int ret = -1;
+qemuDomainObjPrivatePtr priv = vm->privateData;
+virDomainCCWAddressSetPtr ccwaddrs = NULL;
+char *vhostfdName = NULL;
+int vhostfd = -1;
+char *devstr = NULL;
+bool teardowncgroup = false;
+bool teardownlabel = false;
+bool releaseaddr = false;
+
+if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("SCSI passthrough is not supported by this version of 
qemu"));
+goto cleanup;
+}
+
+if (qemuHostdevPrepareHostDevices(driver, vm->def->name, &hostdev, 1) < 0) 
{
+virDomainHostdevSubsysHostPtr hostsrc = &hostdev->source.subsys.u.host;
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Unable to prepare scsi_host hostdev: %s"),
+   hostsrc->wwpn);
+goto cleanup;
+}
+
+if (qemuSetupHostdevCgroup(vm, hostdev) < 0)
+goto cleanup;
+teardowncgroup = true;
+
+if (virSecurityManagerSetHostdevLabel(driver->securityManager,
+  vm->def, hostdev, NULL) < 0)
+goto cleanup;
+teardownlabel = true;
+
+if (virHostOpenVhostSCSI(&vhostfd) < 0)
+goto cleanup;
+
+if (virAsprintf(&vhostfdName, "vhostfd-%d", vhostfd) < 0)
+goto cleanup;
+
+if (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+if (qemuDomainMachineIsS390CCW(vm->def) &&
+virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW))
+hostdev->info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW;
+}
+
+if (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
+hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, hostdev->info) < 0)
+goto cleanup;
+} else if (hostdev->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
+if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def)))
+goto cleanup;
+if (virDomainCCWAddressAssign(hostdev->info, ccwaddrs,
+  !hostdev->info->addr.ccw.assigned) < 0)
+goto cleanup;
+}
+releaseaddr = true;
+
+if (qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1) < 0)
+goto cleanup;
+
+if (!(devstr = qemuBuildHostHostdevDevStr(vm->def,
+  hostdev,
+  priv->qemuCaps,
+  vhostfdName)))
+goto cleanup;
+
+qemuDomainObjEnterMonitor(driver, vm);
+
+ret = qemuMonitorAddDeviceWithFd(priv->mon, devstr, vhostfd, vhostfdName);
+
+if (qemuDomainObjExitMonitor(driver, vm) < 0) {
+ret = -1;
+goto cleanup;
+}
+
+virDomainAuditHostdev(vm, hostdev, "attach", (ret == 0));
+
+if (ret < 0)
+goto cleanup;
+
+if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0)
+goto cleanup;
+
+vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
+
+virDomainCCWAddressSetFree(ccwaddrs);
+
+VIR_FORCE_CLOSE(vhostfd);
+VIR_FREE(vhostfdName);
+VIR_FREE(devstr);
+return 0;
+
+ cleanup:
+if (teardowncgroup && qemuTeardownHostdevCgroup(vm, hostdev) < 0)
+VIR_WARN("Unable to remove host device cgroup ACL on hotplug fail");
+if (teardownlabel &&
+virSecurityManagerRestoreHostdevLabel(driver->securityManager,
+  vm->def, hostdev, NULL) < 0)
+VIR_WARN("Unable to restore host device 

[libvirt] [PATCH v3 9/9] docs: Add vhost-scsi

[Eric Farman]

Signed-off-by: Eric Farman 
---
 docs/formatdomain.html.in | 24 
 1 file changed, 24 insertions(+)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index c70377b..6ef864f 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3694,6 +3694,17 @@
   
   ...
 
+or:
+
+
+  ...
+  
+
+  
+
+  
+  ...
+
 
   hostdev
   The hostdev element is the main container for describing
@@ -3732,6 +3743,12 @@
 If a disk lun in the domain already has the rawio capability,
 then this setting not required.
   
+  scsi_host
+  since 2.5.0For SCSI devices, user
+is responsible to make sure the device is not used by host. This
+type passes all LUNs presented by a single HBA to
+the guest.
+  
 
 
   Note: The managed attribute is only used with PCI 
devices
@@ -3795,6 +3812,13 @@
 credentials to the iSCSI server.
 
   
+  scsi_host
+  Since 2.5.0, multiple LUNs behind a
+single SCSI HBA are described by a protocol
+attribute set to "vhost" and a wwpn attribute that
+is the vhost_scsi wwpn (16 hexadecimal digits with a prefix of
+"naa.") established in the host configfs.
+  
 
   
   vendor, product
-- 
1.9.1

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

[libvirt] [PATCH v3 8/9] tests: Introduce basic vhost-scsi test

[Eric Farman]

These tests were cloned from hostdev-scsi-virtio-scsi in both
xml2argv and xml2xml

Signed-off-by: Eric Farman 
Reviewed-by: Boris Fiuczynski 
---
 .../qemuxml2argv-hostdev-scsi-vhost-scsi.args  | 24 +
 .../qemuxml2argv-hostdev-scsi-vhost-scsi.xml   | 41 ++
 tests/qemuxml2argvtest.c   |  3 ++
 .../qemuxml2xmlout-hostdev-scsi-vhost-scsi.xml |  1 +
 tests/qemuxml2xmltest.c|  3 ++
 5 files changed, 72 insertions(+)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml
 create mode 12 
tests/qemuxml2xmloutdata/qemuxml2xmlout-hostdev-scsi-vhost-scsi.xml

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args 
b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args
new file mode 100644
index 000..5cd2939
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.args
@@ -0,0 +1,24 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest2 \
+-S \
+-M pc \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9466-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest2/monitor.sock,server,nowait \
+-no-acpi \
+-boot c \
+-device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 \
+-usb \
+-drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \
+-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
+-device vhost-scsi-pci,wwpn=naa.5123456789abcde0,vhostfd=3,id=hostdev0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x4
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml 
b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml
new file mode 100644
index 000..4d57fb8
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml
@@ -0,0 +1,41 @@
+
+  QEMUGuest2
+  c7a5fdbd-edaf-9466-926a-d65c16db1809
+  219100
+  219100
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu
+
+  
+  
+  
+
+
+  
+
+
+  
+
+
+  
+
+
+
+
+
+  
+
+
+  
+
+  
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 90d6aaf..1b3f375 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1886,6 +1886,9 @@ mymain(void)
 DO_TEST("hostdev-scsi-virtio-iscsi-auth",
 QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_VIRTIO_SCSI,
 QEMU_CAPS_DEVICE_SCSI_GENERIC);
+DO_TEST("hostdev-scsi-vhost-scsi",
+QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_DEVICE_VHOST_SCSI,
+QEMU_CAPS_DEVICE_SCSI_GENERIC);
 
 DO_TEST("mlock-on", QEMU_CAPS_REALTIME_MLOCK);
 DO_TEST_FAILURE("mlock-on", NONE);
diff --git 
a/tests/qemuxml2xmloutdata/qemuxml2xmlout-hostdev-scsi-vhost-scsi.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-hostdev-scsi-vhost-scsi.xml
new file mode 12
index 000..76ebe4c
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-hostdev-scsi-vhost-scsi.xml
@@ -0,0 +1 @@
+../qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml
\ No newline at end of file
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 496ed13..261b888 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -737,6 +737,9 @@ mymain(void)
 QEMU_CAPS_DEVICE_IOH3420,
 QEMU_CAPS_HDA_DUPLEX);
 
+DO_TEST("hostdev-scsi-vhost-scsi",
+QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_DEVICE_VHOST_SCSI,
+QEMU_CAPS_DEVICE_SCSI_GENERIC);
 DO_TEST("hostdev-scsi-lsi",
 QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_SCSI_LSI,
 QEMU_CAPS_DEVICE_SCSI_GENERIC);
-- 
1.9.1

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

[libvirt] [PATCH v3 1/9] qemu: Introduce vhost-scsi capability

[Eric Farman]

Do all the stuff for the vhost-scsi capability in QEMU,
so it's in place for our checks later.

Signed-off-by: Eric Farman 
Reviewed-by: Boris Fiuczynski 
---
 src/qemu/qemu_capabilities.c| 2 ++
 src/qemu/qemu_capabilities.h| 1 +
 tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml| 1 +
 tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml| 1 +
 tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml| 1 +
 tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml| 1 +
 tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml| 1 +
 tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml| 1 +
 tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml | 1 +
 tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml | 1 +
 tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml   | 1 +
 tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml| 1 +
 tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml| 1 +
 13 files changed, 14 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 7a8202a..d1ce0d9 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -347,6 +347,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "machine-iommu",
   "virtio-vga",
   "drive-iotune-max-length",
+  "vhost-scsi",
 );
 
 
@@ -1588,6 +1589,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
 { "pxb-pcie", QEMU_CAPS_DEVICE_PXB_PCIE },
 { "tls-creds-x509", QEMU_CAPS_OBJECT_TLS_CREDS_X509 },
 { "intel-iommu", QEMU_CAPS_DEVICE_INTEL_IOMMU },
+{ "vhost-scsi", QEMU_CAPS_DEVICE_VHOST_SCSI },
 };
 
 static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 6e7a855..89dddf1 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -381,6 +381,7 @@ typedef enum {
 QEMU_CAPS_MACHINE_IOMMU, /* -machine iommu=on */
 QEMU_CAPS_DEVICE_VIRTIO_VGA, /* -device virtio-vga */
 QEMU_CAPS_DRIVE_IOTUNE_MAX_LENGTH, /* -drive bps_max_length = and friends 
*/
+QEMU_CAPS_DEVICE_VHOST_SCSI, /* -device vhost-scsi-{ccw,pci} */
 
 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml
index cc4ffef..2931418 100644
--- a/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml
@@ -139,6 +139,7 @@
   
   
   
+  
   1005003
   0
   
diff --git a/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml
index 8e0e697..15f2075 100644
--- a/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml
@@ -144,6 +144,7 @@
   
   
   
+  
   1006000
   0
   
diff --git a/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml
index 40acf62..0726443 100644
--- a/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml
@@ -146,6 +146,7 @@
   
   
   
+  
   1007000
   0
   
diff --git a/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml
index a8a79af..498e2d0 100644
--- a/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml
@@ -161,6 +161,7 @@
   
   
   
+  
   2001001
   0
   
diff --git a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
index 3162758..18594e6 100644
--- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
@@ -181,6 +181,7 @@
   
   
   
+  
   2004000
   0
   
diff --git a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml
index 62d42c3..a36136d 100644
--- a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml
@@ -186,6 +186,7 @@
   
   
   
+  
   2005000
   0
   
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml 
b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
index 5c6a709..9ef440f 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
@@ -159,6 +159,7 @@
   
   
   
+  
   2005094
   0
   
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml 
b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
index 6ba97be..25c1995 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
@@ -159,6 +159,7 @@
   
   
   
+  
   2005094
   0
   
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml 
b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml
index 9174f58.

[libvirt] [PATCH v3 2/9] Introduce a "scsi_host" hostdev type

[Eric Farman]

We already have a "scsi" hostdev type, which refers to a single LUN
that is passed through to a guest.  But what of things where multiple
LUNs are passed through via a single SCSI HBA, such as with the
vhost-scsi target?  Create a new hostdev type that will carry this.

Signed-off-by: Eric Farman 
---
 src/conf/domain_conf.c  | 12 +++-
 src/conf/domain_conf.h  | 18 ++
 src/qemu/qemu_cgroup.c  |  7 +++
 src/qemu/qemu_hotplug.c |  2 ++
 src/security/security_apparmor.c|  4 
 src/security/security_dac.c |  8 
 src/security/security_selinux.c |  8 
 tests/domaincapsschemadata/full.xml |  1 +
 8 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7275921..08c4f8b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -647,7 +647,8 @@ VIR_ENUM_IMPL(virDomainHostdevMode, 
VIR_DOMAIN_HOSTDEV_MODE_LAST,
 VIR_ENUM_IMPL(virDomainHostdevSubsys, VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST,
   "usb",
   "pci",
-  "scsi")
+  "scsi",
+  "scsi_host")
 
 VIR_ENUM_IMPL(virDomainHostdevSubsysPCIBackend,
   VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST,
@@ -661,6 +662,11 @@ VIR_ENUM_IMPL(virDomainHostdevSubsysSCSIProtocol,
   "adapter",
   "iscsi")
 
+VIR_ENUM_IMPL(virDomainHostdevSubsysHostProtocol,
+  VIR_DOMAIN_HOSTDEV_SUBSYS_HOST_PROTOCOL_TYPE_LAST,
+  "none",
+  "vhost")
+
 VIR_ENUM_IMPL(virDomainHostdevCaps, VIR_DOMAIN_HOSTDEV_CAPS_TYPE_LAST,
   "storage",
   "misc",
@@ -12997,6 +13003,8 @@ virDomainHostdevDefParseXML(virDomainXMLOptionPtr 
xmlopt,
 break;
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
 break;
+case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST:
+break;
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
 goto error;
 break;
@@ -13880,6 +13888,8 @@ virDomainHostdevMatchSubsys(virDomainHostdevDefPtr a,
 return virDomainHostdevMatchSubsysSCSIiSCSI(a, b);
 else
 return virDomainHostdevMatchSubsysSCSIHost(a, b);
+case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST:
+/* Fall through for now */
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
 return 0;
 }
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 24aa79c..2870035 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -294,6 +294,7 @@ typedef enum {
 VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB,
 VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI,
 VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI,
+VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST,
 
 VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST
 } virDomainHostdevSubsysType;
@@ -368,6 +369,22 @@ struct _virDomainHostdevSubsysSCSI {
 } u;
 };
 
+typedef enum {
+VIR_DOMAIN_HOSTDEV_SUBSYS_HOST_PROTOCOL_TYPE_NONE,
+VIR_DOMAIN_HOSTDEV_SUBSYS_HOST_PROTOCOL_TYPE_VHOST,
+
+VIR_DOMAIN_HOSTDEV_SUBSYS_HOST_PROTOCOL_TYPE_LAST,
+} virDomainHostdevSubsysHostProtocolType;
+
+VIR_ENUM_DECL(virDomainHostdevSubsysHostProtocol)
+
+typedef struct _virDomainHostdevSubsysHost virDomainHostdevSubsysHost;
+typedef virDomainHostdevSubsysHost *virDomainHostdevSubsysHostPtr;
+struct _virDomainHostdevSubsysHost {
+int protocol;
+char *wwpn;
+};
+
 typedef struct _virDomainHostdevSubsys virDomainHostdevSubsys;
 typedef virDomainHostdevSubsys *virDomainHostdevSubsysPtr;
 struct _virDomainHostdevSubsys {
@@ -376,6 +393,7 @@ struct _virDomainHostdevSubsys {
 virDomainHostdevSubsysUSB usb;
 virDomainHostdevSubsysPCI pci;
 virDomainHostdevSubsysSCSI scsi;
+virDomainHostdevSubsysHost host;
 } u;
 };
 
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 1443f7e..ee31d14 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -376,6 +376,10 @@ qemuSetupHostdevCgroup(virDomainObjPtr vm,
 break;
 }
 
+case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST: {
+break;
+}
+
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
 break;
 }
@@ -440,6 +444,9 @@ qemuTeardownHostdevCgroup(virDomainObjPtr vm,
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
 /* nothing to tear down for SCSI */
 break;
+case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST:
+/* nothing to tear down for scsi_host */
+break;
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
 break;
 }
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 9746a06..335252b 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -3510,6 +3510,8 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
 case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
 qemuDomainRemoveSCSIHostDevice(driver, vm, hostdev);
 break;
+case VIR_DOMAIN_HOST

[libvirt] [PATCH v3 3/9] util: Management routines for scsi_host devices

[Eric Farman]

Signed-off-by: Eric Farman 
---
 src/Makefile.am  |   1 +
 src/libvirt_private.syms |  19 +++
 src/util/virhost.c   | 301 +++
 src/util/virhost.h   |  72 
 src/util/virhostdev.c| 155 
 src/util/virhostdev.h|  16 +++
 tests/qemuxml2argvmock.c |   9 ++
 7 files changed, 573 insertions(+)
 create mode 100644 src/util/virhost.c
 create mode 100644 src/util/virhost.h

diff --git a/src/Makefile.am b/src/Makefile.am
index 8ee5567..11b7dc4 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -122,6 +122,7 @@ UTIL_SOURCES =  
\
util/virhash.c util/virhash.h   \
util/virhashcode.c util/virhashcode.h   \
util/virhook.c util/virhook.h   \
+   util/virhost.c util/virhost.h   \
util/virhostcpu.c util/virhostcpu.h util/virhostcpupriv.h \
util/virhostdev.c util/virhostdev.h \
util/virhostmem.c util/virhostmem.h \
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b4210f4..c152e76 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1671,6 +1671,23 @@ virHookInitialize;
 virHookPresent;
 
 
+# util/virhost.h
+virHostDeviceFileIterate;
+virHostDeviceFree;
+virHostDeviceGetName;
+virHostDeviceListAdd;
+virHostDeviceListCount;
+virHostDeviceListDel;
+virHostDeviceListFind;
+virHostDeviceListFindIndex;
+virHostDeviceListGet;
+virHostDeviceListNew;
+virHostDeviceListSteal;
+virHostDeviceNew;
+virHostDeviceSetUsedBy;
+virHostOpenVhostSCSI;
+
+
 # util/virhostdev.h
 virHostdevFindUSBDevice;
 virHostdevManagerGetDefault;
@@ -1678,10 +1695,12 @@ virHostdevPCINodeDeviceDetach;
 virHostdevPCINodeDeviceReAttach;
 virHostdevPCINodeDeviceReset;
 virHostdevPrepareDomainDevices;
+virHostdevPrepareHostDevices;
 virHostdevPreparePCIDevices;
 virHostdevPrepareSCSIDevices;
 virHostdevPrepareUSBDevices;
 virHostdevReAttachDomainDevices;
+virHostdevReAttachHostDevices;
 virHostdevReAttachPCIDevices;
 virHostdevReAttachSCSIDevices;
 virHostdevReAttachUSBDevices;
diff --git a/src/util/virhost.c b/src/util/virhost.c
new file mode 100644
index 000..dd0d982
--- /dev/null
+++ b/src/util/virhost.c
@@ -0,0 +1,301 @@
+/*
+ * virscsi.c: helper APIs for managing scsi_host devices
+ *
+ * Copyright (C) 2016 IBM Corporation
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ *
+ * Authors:
+ * Eric Farman 
+ */
+
+#include 
+#include 
+
+#include "virhost.h"
+#include "virlog.h"
+#include "viralloc.h"
+#include "virerror.h"
+#include "virfile.h"
+#include "virstring.h"
+
+VIR_LOG_INIT("util.host");
+
+#define SYSFS_VHOST_SCSI_DEVICES "/sys/kernel/config/target/vhost/"
+#define VHOST_SCSI_DEVICE "/dev/vhost-scsi"
+
+struct _virUsedByInfo {
+char *drvname; /* which driver */
+char *domname; /* which domain */
+};
+typedef struct _virUsedByInfo *virUsedByInfoPtr;
+
+struct _virHostDevice {
+char *name; /* naa. */
+char *path;
+virUsedByInfoPtr *used_by; /* driver:domain(s) using this dev */
+size_t n_used_by; /* how many domains are using this dev */
+};
+
+struct _virHostDeviceList {
+virObjectLockable parent;
+size_t count;
+virHostDevicePtr *devs;
+};
+
+static virClassPtr virHostDeviceListClass;
+
+static void
+virHostDeviceListDispose(void *obj)
+{
+virHostDeviceListPtr list = obj;
+size_t i;
+
+for (i = 0; i < list->count; i++)
+virHostDeviceFree(list->devs[i]);
+
+VIR_FREE(list->devs);
+}
+
+
+static int
+virHostOnceInit(void)
+{
+if (!(virHostDeviceListClass = virClassNew(virClassForObjectLockable(),
+   "virHostDeviceList",
+   sizeof(virHostDeviceList),
+   virHostDeviceListDispose)))
+return -1;
+
+return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(virHost)
+
+/* For virReportOOMError()  and virReportSystemError() */
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+int
+virHostOpenVhostSCSI(int *vhostfd)
+{
+if (!virFileExists(VHOST_SCSI_DEVICE))
+goto error;
+
+*vhostfd = open(VHOST_SCSI_DEVICE, O_RDWR);
+
+if (*vhostfd < 0) {
+   

[libvirt] [PATCH v3 6/9] conf: Wire up the vhost-scsi connection from/to XML

[Eric Farman]

Signed-off-by: Eric Farman 
---
 docs/schemas/domaincommon.rng | 23 
 src/conf/domain_audit.c   |  7 
 src/conf/domain_conf.c| 81 +--
 3 files changed, 109 insertions(+), 2 deletions(-)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index dba9187..ad5d55d 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3963,6 +3963,7 @@
   
   
   
+  
 
   
 
@@ -4091,6 +4092,28 @@
 
   
 
+  
+
+  scsi_host
+
+
+  
+
+  
+
+  vhost 
+
+  
+  
+
+  (naa\.)[0-9a-fA-F]{16}
+
+  
+
+  
+
+  
+
   
 
   storage
diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index 2decf02..844b3cd 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -392,6 +392,7 @@ virDomainAuditHostdev(virDomainObjPtr vm, 
virDomainHostdevDefPtr hostdev,
 virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb;
 virDomainHostdevSubsysPCIPtr pcisrc = &hostdev->source.subsys.u.pci;
 virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi;
+virDomainHostdevSubsysHostPtr hostsrc = &hostdev->source.subsys.u.host;
 
 virUUIDFormat(vm->def->uuid, uuidstr);
 if (!(vmname = virAuditEncode("vm", vm->def->name))) {
@@ -444,6 +445,12 @@ virDomainAuditHostdev(virDomainObjPtr vm, 
virDomainHostdevDefPtr hostdev,
 }
 break;
 }
+case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST:
+if (VIR_STRDUP_QUIET(address, hostsrc->wwpn) < 0) {
+VIR_WARN("OOM while encoding audit message");
+goto cleanup;
+}
+break;
 default:
 VIR_WARN("Unexpected hostdev type while encoding audit message: 
%d",
  hostdev->source.subsys.type);
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 08c4f8b..79cf32e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2318,6 +2318,9 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def)
 } else {
 VIR_FREE(scsisrc->u.host.adapter);
 }
+} else if (def->source.subsys.type == 
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST) {
+virDomainHostdevSubsysHostPtr hostsrc = &def->source.subsys.u.host;
+VIR_FREE(hostsrc->wwpn);
 }
 break;
 }
@@ -6087,6 +6090,55 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr 
sourcenode,
 return ret;
 }
 
+static int
+virDomainHostdevSubsysHostDefParseXML(xmlNodePtr sourcenode,
+  virDomainHostdevDefPtr def)
+{
+char *protocol = NULL;
+virDomainHostdevSubsysHostPtr hostsrc = &def->source.subsys.u.host;
+
+if ((protocol = virXMLPropString(sourcenode, "protocol"))) {
+hostsrc->protocol =
+virDomainHostdevSubsysHostProtocolTypeFromString(protocol);
+if (hostsrc->protocol <= 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Unknown scsi_host subsystem protocol '%s'"),
+   protocol);
+goto cleanup;
+}
+}
+
+switch ((virDomainHostdevSubsysHostProtocolType) hostsrc->protocol) {
+case VIR_DOMAIN_HOSTDEV_SUBSYS_HOST_PROTOCOL_TYPE_VHOST:
+if (!(hostsrc->wwpn = virXMLPropString(sourcenode, "wwpn"))) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+   _("missing vhost-scsi hostdev source path name"));
+goto cleanup;
+}
+
+if (!STRPREFIX(hostsrc->wwpn, "naa.") ||
+strlen(hostsrc->wwpn) != strlen("naa.") + 16) {
+virReportError(VIR_ERR_XML_ERROR, "%s", _("malformed 'wwpn' 
value"));
+goto cleanup;
+}
+break;
+case VIR_DOMAIN_HOSTDEV_SUBSYS_HOST_PROTOCOL_TYPE_NONE:
+case VIR_DOMAIN_HOSTDEV_SUBSYS_HOST_PROTOCOL_TYPE_LAST:
+virReportError(VIR_ERR_XML_ERROR,
+   _("Invalid hostdev protocol '%s'"),
+   
virDomainHostdevSubsysHostProtocolTypeToString(def->source.subsys.type));
+goto cleanup;
+break;
+}
+
+return 0;
+
+ cleanup:
+VIR_FREE(hostsrc->wwpn);
+VIR_FREE(protocol);
+return -1;
+}
+
 
 static int
 virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
@@ -6211,6 +6263,11 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
 goto error;
 break;
 
+case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST:
+if (virDomainHostdevSubsysHostDefParseXML(sourcenode, def) < 0)
+goto error;
+break;
+
 default:
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("address type='%s' not supported in hostdev 
interfaces"),
@@ -13889,7 +13946,13 @@ virDomainHostdevMatchSubs

Re: [libvirt] on shutdown

[Ruben Kerkhof]

On Wed, Oct 26, 2016 at 5:04 PM, Martin Kletzander  wrote:
> Or have you possibly edited the on-disk saved state of libvirt some time
> back instead of using virsh edit? =)

That could have been it, but no, we have this all automated and never
touch the on-disk xml after a domain has been defined.
My git logs show that I've added  somewhere in 2010, and
indeed, it seems to be a leftover from our migration from Xen's
xmdomain.cfg.

Then I found https://bugzilla.redhat.com/651710 and it turns out it
never had any effect for Xen either :)

Kind regards,

Ruben

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

Re: [libvirt] [PATCH 07/10] qemu: agent: simplify io loop

[John Ferlan]

On 10/04/2016 02:56 AM, Nikolay Shirokovskiy wrote:
> Now we don't need to differentiate error and eof cases in the loop function.
> So let's simplify it radically using goto instead of flags.
> ---
>  src/qemu/qemu_agent.c| 185 
> ++-
>  src/qemu/qemu_agent.h|   2 -
>  src/qemu/qemu_process.c  |  22 +
>  tests/qemumonitortestutils.c |   1 -
>  4 files changed, 83 insertions(+), 127 deletions(-)
> 

I would think it's understood the genesis of this comes from
qemuMonitorIO (tripped me up in my patch 1 review too, but still the
same result)...

Part of me believes whatever happens here could be considered for
qemuMonitorIO too, but that's perhaps a bigger hurdle to jump.

I'm not sure why we want to make Error and EOF be the same thing.

John

I'm stopping here...

> diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
> index ec8d47e..43d78c9 100644
> --- a/src/qemu/qemu_agent.c
> +++ b/src/qemu/qemu_agent.c
> @@ -595,8 +595,8 @@ static void
>  qemuAgentIO(int watch, int fd, int events, void *opaque)
>  {
>  qemuAgentPtr mon = opaque;
> -bool error = false;
> -bool eof = false;
> +void (*errorNotify)(qemuAgentPtr, virDomainObjPtr);
> +virDomainObjPtr vm;
>  
>  virObjectRef(mon);
>  /* lock access to the monitor and protect fd */
> @@ -606,122 +606,95 @@ qemuAgentIO(int watch, int fd, int events, void 
> *opaque)
>  #endif
>  
>  if (mon->fd != fd || mon->watch != watch) {
> -if (events & (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR))
> -eof = true;
>  virReportError(VIR_ERR_INTERNAL_ERROR,
> _("event from unexpected fd %d!=%d / watch %d!=%d"),
> mon->fd, fd, mon->watch, watch);
> -error = true;
> -} else if (mon->lastError.code != VIR_ERR_OK) {
> -if (events & (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR))
> -eof = true;
> -error = true;
> -} else {
> -if (events & VIR_EVENT_HANDLE_WRITABLE) {
> -if (mon->connectPending) {
> -if (qemuAgentIOConnect(mon) < 0)
> -error = true;
> -} else {
> -if (qemuAgentIOWrite(mon) < 0)
> -error = true;
> -}
> -events &= ~VIR_EVENT_HANDLE_WRITABLE;
> -}
> -
> -if (!error &&
> -events & VIR_EVENT_HANDLE_READABLE) {
> -int got = qemuAgentIORead(mon);
> -events &= ~VIR_EVENT_HANDLE_READABLE;
> -if (got < 0) {
> -error = true;
> -} else if (got == 0) {
> -eof = true;
> -} else {
> -/* Ignore hangup/error events if we read some data, to
> - * give time for that data to be consumed */
> -events = 0;
> -
> -if (qemuAgentIOProcess(mon) < 0)
> -error = true;
> -}
> -}
> -
> -if (!error &&
> -events & VIR_EVENT_HANDLE_HANGUP) {
> -virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -   _("End of file from agent monitor"));
> -eof = true;
> -events &= ~VIR_EVENT_HANDLE_HANGUP;
> -}
> -
> -if (!error && !eof &&
> -events & VIR_EVENT_HANDLE_ERROR) {
> -virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -   _("Invalid file descriptor while waiting for 
> monitor"));
> -eof = true;
> -events &= ~VIR_EVENT_HANDLE_ERROR;
> -}
> -if (!error && events) {
> -virReportError(VIR_ERR_INTERNAL_ERROR,
> -   _("Unhandled event %d for monitor fd %d"),
> -   events, mon->fd);
> -error = true;
> -}
> +goto error;
>  }
>  
> -if (error || eof) {
> -if (mon->lastError.code != VIR_ERR_OK) {
> -/* Already have an error, so clear any new error */
> -virResetLastError();
> +if (mon->lastError.code != VIR_ERR_OK)
> +goto error;
> +
> +if (events & VIR_EVENT_HANDLE_WRITABLE) {
> +if (mon->connectPending) {
> +if (qemuAgentIOConnect(mon) < 0)
> +goto error;
>  } else {
> -virErrorPtr err = virGetLastError();
> -if (!err)
> -virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -   _("Error while processing monitor IO"));
> -virCopyLastError(&mon->lastError);
> -virResetLastError();
> +if (qemuAgentIOWrite(mon) < 0)
> +goto error;
>  }
> +events &= ~VIR_EVENT_HANDLE_WRITABLE;
> +}
>  
> -VIR_DEBUG("Error on monitor %s", NULLSTR(mon->lastError.message));
> -/* If IO process resulted in an error & we have a message,
> - * then 

Re: [libvirt] [PATCH 06/10] qemu: agent: drop agent error flag

[John Ferlan]

On 10/04/2016 02:56 AM, Nikolay Shirokovskiy wrote:
> Let's take a closer look at qemuAgentIO. In the case of error
> we stop listening to any events besides error and eof.
> Then set last error so that all next loop invocations do very little:
> 
> 1. if it is an error then just call error callback once more.
> Current callback is noop for subsequent invocations.
> 
> 2. if it is an eof then call eof callback, it will close
> monitor eventually.
> 
> So why waiting for eof? Let's just close monitor on error.
> Now we can drop error flag and deal with NULL monitor
> case only (qemuDomainAgentAvailable stays correct).
> ---
>  src/qemu/qemu_domain.c  |  8 
>  src/qemu/qemu_domain.h  |  1 -
>  src/qemu/qemu_driver.c  |  1 -
>  src/qemu/qemu_process.c | 19 ++-
>  4 files changed, 2 insertions(+), 27 deletions(-)
> 

While we're not "reading" anything - why continue to sent and wait for
more stuff if before sending (e.g. qemuDomainAgentAvailable callers) we
could detect that there was a failure and thus we shouldn't even attempt
the send.

Wouldn't the EOF tell us that we're all done processing whatever was
sent our way before we got the first error?

Not so sure about this one. I'd have to think more about things in light
of what's being chased.

John
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index cd788c8..b6756b1 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -5055,14 +5055,6 @@ qemuDomainAgentAvailable(virDomainObjPtr vm,
>  }
>  return false;
>  }
> -if (priv->agentError) {
> -if (reportError) {
> -virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
> -   _("QEMU guest agent is not "
> - "available due to an error"));
> -}
> -return false;
> -}
>  
>  if (priv->agent)
>  return true;
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 521531b..257bfcb 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -177,7 +177,6 @@ struct _qemuDomainObjPrivate {
>  unsigned long long monStart;
>  
>  qemuAgentPtr agent;
> -bool agentError;
>  unsigned long long agentStart;
>  
>  bool gotShutdown;
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index edff973..b6fb1df 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4455,7 +4455,6 @@ processSerialChangedEvent(virQEMUDriverPtr driver,
>  if (priv->agent) {
>  qemuAgentClose(priv->agent);
>  priv->agent = NULL;
> -priv->agentError = false;
>  }
>  }
>  
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 78d530f..3da1bd5 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -151,7 +151,6 @@ qemuProcessHandleAgentEOF(qemuAgentPtr agent,
>  
>  qemuAgentClose(agent);
>  priv->agent = NULL;
> -priv->agentError = false;
>  
>  virObjectUnlock(vm);
>  return;
> @@ -169,21 +168,10 @@ qemuProcessHandleAgentEOF(qemuAgentPtr agent,
>   * allowed
>   */
>  static void
> -qemuProcessHandleAgentError(qemuAgentPtr agent ATTRIBUTE_UNUSED,
> +qemuProcessHandleAgentError(qemuAgentPtr agent,
>  virDomainObjPtr vm)
>  {
> -qemuDomainObjPrivatePtr priv;
> -
> -VIR_DEBUG("Received error from agent on %p '%s'", vm, vm->def->name);
> -
> -virObjectLock(vm);
> -
> -priv = vm->privateData;
> -
> -if (priv->agent)
> -priv->agentError = true;
> -
> -virObjectUnlock(vm);
> +qemuProcessHandleAgentEOF(agent, vm);
>  }
>  
>  static void qemuProcessHandleAgentDestroy(qemuAgentPtr agent,
> @@ -209,8 +197,6 @@ qemuConnectAgent(virQEMUDriverPtr driver, virDomainObjPtr 
> vm)
>  qemuAgentPtr agent = NULL;
>  virDomainChrDefPtr config = qemuFindAgentConfig(vm->def);
>  
> -priv->agentError = false;
> -
>  if (!config)
>  return 0;
>  
> @@ -5915,7 +5901,6 @@ void qemuProcessStop(virQEMUDriverPtr driver,
>  if (priv->agent) {
>  qemuAgentClose(priv->agent);
>  priv->agent = NULL;
> -priv->agentError = false;
>  }
>  
>  if (priv->mon) {
> 

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

Re: [libvirt] [PATCH 05/10] qemu: agent: straigten up failed agent start case

[John Ferlan]

On 10/04/2016 02:56 AM, Nikolay Shirokovskiy wrote:
> agentError is used for 2 different cases:
> 
> 1. agent monitor is failed to start

Non guest fatal failure in qemuConnectAgent when first trying to connect
to the agent

> 2. io error in agent monitor

I/O error with running agent resulting in qemuProcessHandleAgentError
being called

Most of the above would seemingly be a reason for the removal of the
agentError (which needs to be a separate patch)...

> 
> Actually to check for the first case we don't need the
> flag at all. NULL agent is always error for old qemu
> (QEMU_CAPS_VSERPORT_CHANGE is not supported), while
> for modern qemu it is an error only if channel is in
> connected state.

I suppose this explanation could be added too, but I'd have to see it in
the context of what I've learned so far in a "new" series.

In any case, none of the above text seems to have anything to do with
the change in reporting the errors at startup.

> ---
>  src/qemu/qemu_domain.c  | 37 +++--
>  src/qemu/qemu_process.c |  1 -
>  2 files changed, 23 insertions(+), 15 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 9b1a32e..cd788c8 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -5046,6 +5046,7 @@ qemuDomainAgentAvailable(virDomainObjPtr vm,
>   bool reportError)
>  {
>  qemuDomainObjPrivatePtr priv = vm->privateData;
> +virDomainChrDefPtr config;
>  
>  if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_RUNNING) {
>  if (reportError) {
> @@ -5062,22 +5063,30 @@ qemuDomainAgentAvailable(virDomainObjPtr vm,
>  }
>  return false;
>  }
> -if (!priv->agent) {
> -if (qemuFindAgentConfig(vm->def)) {
> -if (reportError) {
> -virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
> -   _("QEMU guest agent is not connected"));
> -}
> -return false;
> -} else {
> -if (reportError) {
> -virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> -   _("QEMU guest agent is not configured"));
> -}
> -return false;
> +
> +if (priv->agent)
> +return true;
> +
> +config = qemuFindAgentConfig(vm->def);
> +
> +if (!config) {
> +if (reportError) {
> +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
> +   _("QEMU guest agent is not configured"));
>  }

I'm OK through here, but the next two I'm not so sure I agree with.

> +} else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VSERPORT_CHANGE) &&
> +   config->state != VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED) {

What does the manner of startup (either legacy or VSERPORT) have to do
with which error should be reported.
> +if (reportError) {
> +virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
> +   _("QEMU guest agent is not connected"));
> +}
> +} else if (reportError) {
> +virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
> +   _("QEMU guest agent is not "
> + "available due to an error"));

I know you're setting up for the next set of patches (to remove the
agentError flag), but I'm not sure yet whether using VSERPORT change
configuration is the right way.

Unless of course my assumption from patch 1 review is true... This is
really a startup/shutdown race...

John
>  }
> -return true;
> +
> +return false;
>  }
>  
>  
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index d7c9ce3..78d530f 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -267,7 +267,6 @@ qemuConnectAgent(virQEMUDriverPtr driver, virDomainObjPtr 
> vm)
>   cleanup:
>  if (!priv->agent) {
>  VIR_WARN("Cannot connect to QEMU guest agent for %s", vm->def->name);
> -priv->agentError = true;

Wait, what, why?  you just added this in the previous patch and I see no
reason for removing it now.

This would need a separate patch and reason.

John

>  virResetLastError();
>  }
>  
> 

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

Re: [libvirt] [PATCH 04/10] qemu: agent: handle agent connection errors in one place

[John Ferlan]

There's no commit message...

You're altering qemuConnectAgent to return -1 on the only error and
perform the VIR_WARN plus agentError = true on other "soft" failures.

Not exactly nothing going on!

On 10/04/2016 02:56 AM, Nikolay Shirokovskiy wrote:
> ---
>  src/qemu/qemu_driver.c|  8 +--
>  src/qemu/qemu_migration.c | 13 ++-
>  src/qemu/qemu_process.c   | 58 
> ---
>  3 files changed, 17 insertions(+), 62 deletions(-)
> 

This idea seems to have merit too - something that I've thought now that
I've read the first 3 patches...

I think you should have started with this patch, it probably would have
made the others easier to think about. In fact, I'm curious if with just
this change and the suggestion in patch 3 for clearing agentError
whether you can reproduced the bug you're trying to fix/resolve without
any of the other patches.

> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 12ddbc0..edff973 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4397,7 +4397,6 @@ processSerialChangedEvent(virQEMUDriverPtr driver,
>  virObjectEventPtr event = NULL;
>  virDomainDeviceDef dev;
>  qemuDomainObjPrivatePtr priv = vm->privateData;
> -int rc;
>  
>  if (connected)
>  newstate = VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED;
> @@ -4450,13 +4449,8 @@ processSerialChangedEvent(virQEMUDriverPtr driver,
>  
>  if (STREQ_NULLABLE(dev.data.chr->target.name, "org.qemu.guest_agent.0")) 
> {
>  if (newstate == VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED) {
> -if (!priv->agent) {
> -if ((rc = qemuConnectAgent(driver, vm)) == -2)
> +if (!priv->agent && qemuConnectAgent(driver, vm) < 0)

FWIW:
qemuConnectAgent returns 0 "if (priv->agent)", so there's one less check
here too.  Could just be "if (qemuConnectAgent(driver, vm) < 0)"

>  goto endjob;

The indention for this is off (remove leading 4 spaces)

> -
> -if (rc < 0)
> -priv->agentError = true;
> -}
>  } else {
>  if (priv->agent) {
>  qemuAgentClose(priv->agent);
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index e2ca330..0a02236 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -6171,7 +6171,6 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
>  unsigned short port;
>  unsigned long long timeReceived = 0;
>  virObjectEventPtr event;
> -int rc;
>  qemuDomainJobInfoPtr jobInfo = NULL;
>  bool inPostCopy = false;
>  bool doKill = true;
> @@ -6244,16 +6243,8 @@ qemuMigrationFinish(virQEMUDriverPtr driver,
>QEMU_ASYNC_JOB_MIGRATION_IN) < 0)
>  goto endjob;
>  
> -if ((rc = qemuConnectAgent(driver, vm)) < 0) {
> -if (rc == -2)
> -goto endjob;
> -
> -VIR_WARN("Cannot connect to QEMU guest agent for %s",
> - vm->def->name);
> -virResetLastError();
> -priv->agentError = true;
> -}
> -
> +if (qemuConnectAgent(driver, vm) < 0)
> +goto endjob;
>  
>  if (flags & VIR_MIGRATE_PERSIST_DEST) {
>  if (qemuMigrationPersist(driver, vm, mig, !v3proto) < 0) {
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 42f7f84..d7c9ce3 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -206,7 +206,6 @@ int
>  qemuConnectAgent(virQEMUDriverPtr driver, virDomainObjPtr vm)
>  {
>  qemuDomainObjPrivatePtr priv = vm->privateData;
> -int ret = -1;
>  qemuAgentPtr agent = NULL;
>  virDomainChrDefPtr config = qemuFindAgentConfig(vm->def);
>  
> @@ -252,8 +251,7 @@ qemuConnectAgent(virQEMUDriverPtr driver, virDomainObjPtr 
> vm)
>  qemuAgentClose(agent);
>  virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("guest crashed while connecting to the guest 
> agent"));
> -ret = -2;
> -goto cleanup;
> +return -1;
>  }
>  
>  if (virSecurityManagerClearSocketLabel(driver->securityManager,
> @@ -264,18 +262,16 @@ qemuConnectAgent(virQEMUDriverPtr driver, 
> virDomainObjPtr vm)
>  goto cleanup;
>  }
>  
> -
>  priv->agent = agent;
>  
> -if (priv->agent == NULL) {
> -VIR_INFO("Failed to connect agent for %s", vm->def->name);

Interesting a "marker" of sorts to know the virAgentOpen "failed" is
that we'd get an Informational "Failed to connect..." followed shortly
thereafter by a Warning "Cannot connect..." (depending of course on
one's message display severity level).

I think if you "restore" this without the goto cleanup (below) and of
course the removal of the {}, then at least message parity is
achieved...  I suppose it could move up into the "if (agent == NULL)"
too, but then it could be given even though an Error is reported.

It's not that important, but it does

Re: [libvirt] [PATCH 03/10] qemu: agent: clear error flag upon init

[John Ferlan]

On 10/04/2016 02:56 AM, Nikolay Shirokovskiy wrote:
> There were a few errors in the code when this flag was not
> cleared upon monitor cleanup. All of them could be fixed
> just resetting this flag upon agent monitor initialization.

We should fix the places where the flag wasn't cleared properly then.

> ---
>  src/qemu/qemu_process.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 3637f4b..42f7f84 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -210,6 +210,8 @@ qemuConnectAgent(virQEMUDriverPtr driver, virDomainObjPtr 
> vm)
>  qemuAgentPtr agent = NULL;
>  virDomainChrDefPtr config = qemuFindAgentConfig(vm->def);
>  
> +priv->agentError = false;
> +

This idea actually may have some merit and may actually be the cause of
the bug you saw, but not right here.

>  if (!config)
>  return 0;
>  
> 

I think perhaps a better place would be some time after we ascertain
that "priv->agent" is not already set since the only way to set it is as
a result of this function.

My first inclination was in that VSERPORT_CHANGE check and return 0
(e.g. if called from qemuMigrationFinish, qemuProcessReconnect,
qemuProcessLaunch, or qemuProcessAttach).

But then I wasn't sure if there would be a race between setting 'state'
to VIR_DOMAIN_CHR_DEVICE_STATE_CONNECTED in processSerialChangedEvent
and one of the 4 other paths.

So, I think perhaps the better place would be after the virObjectRef(vm)
prior to calling qemuAgentOpen and unlocking the vm. IOW: We have the
lock, alter priv->agentError.  We're about to start anyway...

That way, we'd know we're about the unconditionally clear the agentError
just as if we were starting the first time, reconnecting to a running
domain, migrating, or attaching.

Thus ignoring patch 1 and 2 for a moment and considering this patch
alone, we'd know for sure the agentError was cleared before a "real"
startup and after a qemuProcessStop.  Thus clearing before we start
seems to be right (although it makes me wonder how it could have been set).

Personally, I'd start with applying patch 4 and then trying to reproduce
the condition... Have some sort of "marker" here that checks if
agentError is true then complain.  If we can figure out how we get into
this function with agentError being set, then I think that'll really
help me understand the order of events...


John

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

Re: [libvirt] [PATCH 02/10] qemu: agent: dont set error after agent cleanup

[John Ferlan]

On 10/04/2016 02:56 AM, Nikolay Shirokovskiy wrote:
> If there is an error event after eof event then after agent
> is cleaned up on eof error flag will be set back and remains
> set after next domain start up making agent unavailable.
> Thus let's check before set this flag on error event.
> ---
>  src/qemu/qemu_process.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index a581f96..3637f4b 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -180,7 +180,8 @@ qemuProcessHandleAgentError(qemuAgentPtr agent 
> ATTRIBUTE_UNUSED,
>  
>  priv = vm->privateData;
>  
> -priv->agentError = true;
> +if (priv->agent)
> +priv->agentError = true;

A NULL priv->agent happens for the following reasons:

1. qemuDomainObjExitAgent - when the last reference is removed... I
would hope we're not falling into this path for what you've seen...

2. processSerialChangedEvent - after a qemuAgentClose for VSERPORT
callback and agentError is set to false... (so that seems to be
happening properly)

3. qemuProcessHandleAgentEOF (discussed in patch 1) - where I don't
believe you should be clearing agentError

4. qemuConnectAgent has a failure after qemuAgentOpen creates the
'agent', but before sets priv->agent = agent.

5. qemuProcessStop - stops a started agent *and* clears agentError flag
(so that seems to be happening properly).

w/r/t #4... there are windows in qemuConnectAgent where some error after
qemuAgentOpen *succeeds* could trigger this function to be called before
priv->agent is filled in.

Thus by only clearing it when priv->agent is set, we could miss some
error that occurred.  While the "next" call could also fail and
eventually set the error, I'm not convinced that only clearing when
priv->agent is set is right.

Long way of saying NAK on this one.

John
>  
>  virObjectUnlock(vm);
>  }
> 

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

Re: [libvirt] [PATCH 01/10] qemu: agent: unset error flag in EOF handler

[John Ferlan]

On 10/04/2016 02:56 AM, Nikolay Shirokovskiy wrote:
> Usually on domain shutdown event comes first from qemu
> and the flag is unset in processSerialChangedEvent. However
> there is a race between this function and qemuProcessHandleAgentEOF
> because the former is executed in a thread pool and the latter
> executed synchronously. qemuProcessHandleAgentEOF do not unset
> the flag thus if it was triggered before it remains set after domain
> shutdown/start and thus qemu agent becomes unavailable.


What error occurred that would be avoided by clearing the agentError
flag unconditionally? Considering that agentError being set is primarily
used by "new connection" request processing (e.g. callers to
qemuDomainAgentAvailable) in order to ascertain whether they should
actually attempt the connection or fail because of some condition that
would eventually lead to failure.  I would think EOF from qemu_monitor
for the qemu_agent would be one of those conditions *before* the domain
state is not RUNNING or the priv->agent has been fully removed.

I think what I'm missing is what sequence of events led to this
condition? How was it reproduced.

> ---
>  src/qemu/qemu_process.c | 1 +
>  1 file changed, 1 insertion(+)
> 

So I understand "eventually" your goal is to remove agentError in patch
6, but you need to lead down the path to get there. So let's see if I
can follow along...

The issue you chased is related to domain shutdown processing "order".

The purpose up to this point in time for 'agentError' is to ensure
callers of qemuDomainAgentAvailable() will fail before trying to use the
agent. The qemuDomainAgentAvailable checks domain active, agentError,
and whether the priv->agent is available.

So the contention is during shutdown the EOF event firing at about the
same time as the processSerialChangedEvent has caused an issue, but it's
not 100% clear just from the description the sequence of events and the
issue. So let's consider why these things happen:

1. The 'eofNotify' comes from the qemuMonitorIO function - ostensibly if
the guest is shutting down

2. The 'processSerialChangedEvent' seems to occur for multiple reasons -
startup, shutdown, some sort of channel failure (unix socket or pty). If
I'm reading things correctly, this path is only involved when VSERPORT
is being used.

So, is this a case of guest startup where the initial agent
connect/startup occurs (VSERPORT) and a guest shutdown racing? Or is
this purely a shutdown ordering thing?

> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 31c8453..a581f96 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -151,6 +151,7 @@ qemuProcessHandleAgentEOF(qemuAgentPtr agent,
>  
>  qemuAgentClose(agent);
>  priv->agent = NULL;

Because this is cleared, then qemuDomainAgentAvailable will fail because
the agent is not present instead of failing with there was some error.
That would seem to make the following clearing "safe" since we know
we'll have a failure any way, but...

> +priv->agentError = false;

How does this alone solve the problem you saw.  AFAICT it just changes
*where* the error occurs and the error message displayed.

Conversely, if for some reason for either

1. qemuConnectAgent failed and is being "flagged" in
  1a. processSerialChangedEvent (for initial CONNECTED failure when
QEMU_CAPS_VSERPORT_CHANGE)
  1b. qemuMigrationFinish (for migration target)
  1c. qemuProcessReconnect (for libvirtd restart)
  1d. qemuProcessLaunch (initial domain startup before
QEMU_CAPS_VSERPORT_CHANGE)
  1e. qemuProcessAttach (for qemu-attach to existing qemu process)

or

2. qemuProcessHandleAgentError - some sort of error and we're stopping
any further agent communication.

then you're potentially clearing agentError and could conceivably cause
"extra" debugging steps to be taken only to figure out the reason it was
because of some error rather than the "next" connection failing and
indicating the failure was because of some previous error (it's a
startup race for some other thread desiring to use the agent).

>  
>  virObjectUnlock(vm);
>  return;
> 

So given all that - I'm not 100% clear how this really fixes the issue
you say.  Although, I have to wonder if the bug you're chasing is
perhaps that qemuConnectAgent doesn't clear agentError at the right time
as patch 3 attempts to fix.

After reading through patch 4 - I think Patch 4 and a slightly modified
patch 3 will fix what you're seeing, but I'd only know for sure given
more details about what was seen.


John

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

Re: [libvirt] [PATCH] Resuming running domain raise error message

[Martin Kletzander]

On Wed, Oct 26, 2016 at 05:11:58PM +0200, Sławek Kapłoński wrote:

Hello,

Thx a lot but strictly speaking it's my second patch to libvirt :)



I now see it's a fourth one in the three, I have no idea why git log
hadn't showed me any other, maybe I was too fast when dismissing it.
Well, anyway, good that it's in ;)


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

Re: [libvirt] [PATCH 0/5] wireshark: Fix $(prefix) handling

[Andrea Bolognani]

On Wed, 2016-10-26 at 16:54 +0200, Michal Privoznik wrote:
> On 26.10.2016 15:27, Andrea Bolognani wrote:
> > 
> > Well, almost :) There are still some cases that are not
> > handled correctly, but at least this will unbreak 'make
> > rpm' while I work on the rest.
> > 
> > Tested by running 'make rpm' successfully on Fedora 23,
> > Fedora 24 and Fedora rawhide.
> > 
> > 
> > Andrea Bolognani (5):
> >   wireshark: Introduce $ws_modversion
> >   wireshark: Hoist $ws_prefix declaration
> >   wireshark: Strip prefix correctly
> >   wireshark: Inject $(prefix) at the right time
> >   wireshark: Rename plugindir to ws_plugindir
> > 
> >  m4/virt-wireshark.m4 | 29 ++---
> >  tools/Makefile.am|  2 +-
> >  2 files changed, 19 insertions(+), 12 deletions(-)
> 
> ACK series. Thanks for cleaning up my mess.

Don't mention it, it was fun :)

On the other hand, if you really want to thank me, I guess
taking a look at the follow-up series

  https://www.redhat.com/archives/libvir-list/2016-October/msg01187.html

would be as good a way as any :P

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[libvirt] [PATCH 3/3] wireshark: Use ${exec_prefix} for $ws_plugindir

[Andrea Bolognani]

${exec_prefix} and ${prefix} point to the same directory in
most setups, but when that's not the case the former should
be used for architecture-dependent data such as shared objects,
which makes it the best fit for our Wireshark dissector.

While at it, change from $(var) to ${var}: they are absolutely
identicaly as far as make's concerned, but autoconf itself
seems to prefer the latter form so we might as well follow suit.
---
 m4/virt-wireshark.m4 | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/m4/virt-wireshark.m4 b/m4/virt-wireshark.m4
index 75786de..7a817f6 100644
--- a/m4/virt-wireshark.m4
+++ b/m4/virt-wireshark.m4
@@ -52,10 +52,10 @@ AC_DEFUN([LIBVIRT_CHECK_WIRESHARK],[
 ws_prefix="/usr"
   fi
   dnl Replace the wireshark prefix with our own.
-  dnl Note that $(prefix) is kept verbatim at this point in time, and will
-  dnl only be expanded later, when make is called: this makes it possible
-  dnl to override the prefix at compilation or installation time
-  ws_plugindir='$(prefix)'"${ws_plugindir#$ws_prefix}"
+  dnl Note that ${exec_prefix} is kept verbatim at this point in time, and
+  dnl will only be expanded later, when make is called: this makes it
+  dnl possible to override such prefix at compilation or installation time
+  ws_plugindir='${exec_prefix}'"${ws_plugindir#$ws_prefix}"
 elif test "x$with_ws_plugindir" = "xno" || test "x$with_ws_plugindir" = 
"xyes"; then
   AC_MSG_ERROR([ws-plugindir must be used only with valid path])
 else
-- 
2.7.4

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

[libvirt] [PATCH 0/3] wireshark: Further build system fixes

[Andrea Bolognani]

This takes care of the few remaining nits.

All use cases I could think of are covered; if any more
issues are discovered, we'll take care of them then.


Andrea Bolognani (3):
  wireshark: Don't redefine ws_plugindir
  wireshark: Try a bunch of possible prefixes
  wireshark: Use ${exec_prefix} for $ws_plugindir

 m4/virt-wireshark.m4 | 23 ---
 tools/Makefile.am|  1 -
 2 files changed, 16 insertions(+), 8 deletions(-)

-- 
2.7.4

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

[libvirt] [PATCH 2/3] wireshark: Try a bunch of possible prefixes

[Andrea Bolognani]

If we can't obtain Wireshark's plugindir variable from
pkg-config, we fall back to building it ourselves starting
from $libdir.

The problem with that is that we have zero insights on what
$libdir actually looks like, so we can't simply strip $prefix
and call it a day. On the other hand, we have to do *something*
or $ws_plugindir will be unusable.

Our solution is to try the four most likely prefixes, and use
the first one that matches. It's not perfect, but should be
able to cope with all but the weirdest setups.

Worst case scenario, the user can pass --with-ws-plugindir to
configure and explicitly provide a suitable installation path.
---
 m4/virt-wireshark.m4 | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/m4/virt-wireshark.m4 b/m4/virt-wireshark.m4
index 556272a..75786de 100644
--- a/m4/virt-wireshark.m4
+++ b/m4/virt-wireshark.m4
@@ -35,11 +35,20 @@ AC_DEFUN([LIBVIRT_CHECK_WIRESHARK],[
 dnl On some systems the plugindir variable may not be stored within 
pkg config.
 dnl Fall back to older style of constructing the plugin dir path.
 ws_plugindir="$libdir/wireshark/plugins/$ws_modversion"
-ws_prefix="$prefix"
+dnl We have no idea what the contents of $libdir look like, so we'll
+dnl have to play a bit of a guessing game: let's try stripping off
+dnl a bunch of likels prefixed and pick the first one that matches.
+dnl Even if none does, we'll still have one last shot later
+for try in "$prefix" "$exec_prefix" '${prefix}' '${exec_prefix}'; do
+  if test "x${ws_plugindir#$try}" != "x$ws_plugindir"; then
+ws_prefix="$try"
+break
+  fi
+done
   fi
   if test "x$ws_prefix" = "x" ; then
-dnl If the wireshark prefix cannot be retrieved from pkg-config,
-dnl /usr is our best bet
+dnl If the wireshark prefix cannot be retrieved from pkg-config
+dnl or otherwise guessed, /usr is our best bet
 ws_prefix="/usr"
   fi
   dnl Replace the wireshark prefix with our own.
-- 
2.7.4

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

[libvirt] [PATCH 1/3] wireshark: Don't redefine ws_plugindir

[Andrea Bolognani]

autoconf already defines the variable for us, and prints out
a warning if we try to do it a second time. So let's not :)
---
 tools/Makefile.am | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/Makefile.am b/tools/Makefile.am
index 319abb2..100e657 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -398,7 +398,6 @@ EXTRA_DIST += \
 
 if WITH_WIRESHARK_DISSECTOR
 
-ws_plugindir = @ws_plugindir@
 ws_plugin_LTLIBRARIES = wireshark/src/libvirt.la
 wireshark_src_libvirt_la_CPPFLAGS = \
-I wireshark/src $(WIRESHARK_DISSECTOR_CFLAGS)
-- 
2.7.4

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

Re: [libvirt] [PATCH 0/5] wireshark: Fix $(prefix) handling

[Viktor Mihajlovski]

On 26.10.2016 15:27, Andrea Bolognani wrote:
> Well, almost :) There are still some cases that are not
> handled correctly, but at least this will unbreak 'make
> rpm' while I work on the rest.
> 
> Tested by running 'make rpm' successfully on Fedora 23,
> Fedora 24 and Fedora rawhide.
> 
> 
> Andrea Bolognani (5):
>   wireshark: Introduce $ws_modversion
>   wireshark: Hoist $ws_prefix declaration
>   wireshark: Strip prefix correctly
>   wireshark: Inject $(prefix) at the right time
>   wireshark: Rename plugindir to ws_plugindir
> 
>  m4/virt-wireshark.m4 | 29 ++---
>  tools/Makefile.am|  2 +-
>  2 files changed, 19 insertions(+), 12 deletions(-)
> 

Thanks for the fixes. As it happens, I have tried to fix our local
builds in parallel, and thus have some minor suggestions, patch attached.

-- 

Mit freundlichen Grüßen/Kind Regards
   Viktor Mihajlovski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

>From 50ce169ed1c2fa30d2b8f13d6b1dbbd126a9b496 Mon Sep 17 00:00:00 2001
From: Viktor Mihajlovski 
Date: Wed, 26 Oct 2016 17:16:32 +0200
Subject: [PATCH] wireshark: minor improvements for plugin directory
 detecion/guessing

1. If libvirt's and wiresharks' libdirs are different (they
   were on my system) the plugin install can fail. Fixed by evaluating
   the pkg-config libdir variable.
2. May seem pedantic, but the default for libdir is ${exec_prefix}/lib
   and - while uncommon - exec_prefix might be different from prefix.
   So use exec_prefix instead of prefix.
3. AC_SUBST will take care of inserting ws_plugindir = @ws_plugindir@ into
   Makefile.in, so that line can be removed.

Signed-off-by: Viktor Mihajlovski 
---
 m4/virt-wireshark.m4 | 20 ++--
 tools/Makefile.am|  1 -
 2 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/m4/virt-wireshark.m4 b/m4/virt-wireshark.m4
index 556272a..9660c22 100644
--- a/m4/virt-wireshark.m4
+++ b/m4/virt-wireshark.m4
@@ -29,24 +29,24 @@ AC_DEFUN([LIBVIRT_CHECK_WIRESHARK],[
   if test "x$with_wireshark_dissector" != "xno" ; then
 if test "x$with_ws_plugindir" = "xcheck" ; then
   ws_plugindir="$($PKG_CONFIG --variable plugindir wireshark)"
-  ws_prefix="$($PKG_CONFIG --variable prefix wireshark)"
+  ws_libdir="$($PKG_CONFIG --variable libdir wireshark)"
+  ws_exec_prefix="$($PKG_CONFIG --variable exec_prefix wireshark)"
   ws_modversion="$($PKG_CONFIG --modversion wireshark)"
   if test "x$ws_plugindir" = "x" ; then
 dnl On some systems the plugindir variable may not be stored within pkg config.
 dnl Fall back to older style of constructing the plugin dir path.
-ws_plugindir="$libdir/wireshark/plugins/$ws_modversion"
-ws_prefix="$prefix"
+ws_plugindir="$ws_libdir/wireshark/plugins/$ws_modversion"
   fi
-  if test "x$ws_prefix" = "x" ; then
-dnl If the wireshark prefix cannot be retrieved from pkg-config,
+  if test "x$ws_exec_prefix" = "x" ; then
+dnl If the wireshark exec_prefix cannot be retrieved from pkg-config,
 dnl /usr is our best bet
-ws_prefix="/usr"
+ws_exec_prefix="/usr"
   fi
-  dnl Replace the wireshark prefix with our own.
-  dnl Note that $(prefix) is kept verbatim at this point in time, and will
+  dnl Replace the wireshark exec_prefix with our own.
+  dnl Note that $(exec_prefix) is kept verbatim at this point in time, and will
   dnl only be expanded later, when make is called: this makes it possible
-  dnl to override the prefix at compilation or installation time
-  ws_plugindir='$(prefix)'"${ws_plugindir#$ws_prefix}"
+  dnl to override the exec_prefix at compilation or installation time
+  ws_plugindir='$(exec_prefix)'"${ws_plugindir#$ws_exec_prefix}"
 elif test "x$with_ws_plugindir" = "xno" || test "x$with_ws_plugindir" = "xyes"; then
   AC_MSG_ERROR([ws-plugindir must be used only with valid path])
 else
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 319abb2..100e657 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -398,7 +398,6 @@ EXTRA_DIST += \
 
 if WITH_WIRESHARK_DISSECTOR
 
-ws_plugindir = @ws_plugindir@
 ws_plugin_LTLIBRARIES = wireshark/src/libvirt.la
 wireshark_src_libvirt_la_CPPFLAGS = \
 	-I wireshark/src $(WIRESHARK_DISSECTOR_CFLAGS)
-- 
1.9.1

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

Re: [libvirt] libvirt compiles on RISC-V (RV64G)

[Richard W.M. Jones]

On Wed, Oct 26, 2016 at 05:29:26PM +0200, Andrea Bolognani wrote:
> On Wed, 2016-10-26 at 12:35 +0100, Richard W.M. Jones wrote:
> > I'm happy to announce that libvirt compiles fine from git on
> > Fedora/RISC-V.  This has little or no practical value at all, since
> > RISC-V lacks such essentials such as virtualization, qemu etc.
> > However I suppose you could use it as a remote client.
> 
> Did you actually try connecting to a remote libvirtd instance
> from the RISC-V machine?

No, the qemu emulation has no networking (real hardware will of course
have networking).

> Does the test suite pass?

I didn't try it.  It's not massively important that libvirt actually
works until the hypervisor specification is sorted out, and real
hardware is widely available.  Mainly the team want libvirt in order
just to satify deps required to build other packages.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

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

Re: [libvirt] [PATCH 11/17] util: Introduce libvirt_udevhelper

[Daniel P. Berrange]

On Wed, Oct 26, 2016 at 02:36:58PM +0200, Michal Privoznik wrote:
> This is a small helper intended to be run by udev. On its input
> (either as the only command line argument or in DEVNODE
> environment vairable) it is given a device and on the output it
> will either put nothing (meaning the device is not used by any of
> the libvirt domains), or it will print out security labels in the
> following form:
> 
>   UID GID SELABEL

How is this intended to be actually used ? ie what udev rule are
you creating along with this ?  IMHO we just want the helper to
indicate that udev should not do anything to the device - we should
not need udev to ever set labels itself as libvirt has already set
them - we just don't want udev to remove them. IOW, I don't see the
need to print out this info at all.


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

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

Re: [libvirt] [PATCH 07/17] virudev: Introduce virUdevMgrDump

[Daniel P. Berrange]

On Wed, Oct 26, 2016 at 02:36:54PM +0200, Michal Privoznik wrote:
> Now that we are able to store security labels for devices, next
> step is to flush them into a file. For more convenience I've
> chosen JSON format (as we have all the APIs needed for processing
> the format).

I wonder if we're better off avoiding using a structured file
format entirely.

IIUC, we essentially just need a boolean indicator for the udev
helper against each device. Could we do this with a zero length
file per device. eg consider a device '/dev/sda1' That has a
sha256 checksum d8d8d2c47b672edb91407c0014286b472673b2ff9d6636fd567d11650986ba3c
so we could just touch a zero length file

 /var/lib/libvirt/udev/$CHECKSUM

that way the udev helper doesn't need to parse anything - it merely
needs to see if the file exists or not.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|

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

Re: [libvirt] libvirt compiles on RISC-V (RV64G)

[Andrea Bolognani]

On Wed, 2016-10-26 at 12:35 +0100, Richard W.M. Jones wrote:
> I'm happy to announce that libvirt compiles fine from git on
> Fedora/RISC-V.  This has little or no practical value at all, since
> RISC-V lacks such essentials such as virtualization, qemu etc.
> However I suppose you could use it as a remote client.

Did you actually try connecting to a remote libvirtd instance
from the RISC-V machine? Does the test suite pass?

Anyway, awesome news! Thanks for sharing, and keep up the good
work :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [PATCH] Resuming running domain raise error message

[Sławek Kapłoński]

Hello,

Thx a lot but strictly speaking it's my second patch to libvirt :)

-- 
Best regards / Pozdrawiam
Sławek Kapłoński
sla...@kaplonski.pl

On Wed, 26 Oct 2016, Martin Kletzander wrote:

> On Sat, Oct 22, 2016 at 12:30:01PM +0200, Sławek Kapłoński wrote:
> > When user tries to resume already running domain (Qemu or LXC)
> > there is VIR_ERR_OPERATION_INVALID error raised now with
> > message that domain is already running.
> > 
> > Closes-Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1009008
> > ---
> > src/lxc/lxc_driver.c   | 8 +++-
> > src/qemu/qemu_driver.c | 4 
> > 2 files changed, 11 insertions(+), 1 deletion(-)
> > 
> 
> I wanted to split this into two commits, separate it for qemu and lxc,
> but it's so small that when someone were to use this patch, it will fix
> both qemu and lxc and most likely not cause any merge conflicts =)
> 
> Congratulations on your first patch.  I'll slightly reword the commit
> message and push this.
> 
> Martin



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



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

Re: [libvirt] on shutdown

[Martin Kletzander]

On Wed, Oct 26, 2016 at 04:40:12PM +0200, Ruben Kerkhof wrote:

On Thu, Oct 20, 2016 at 7:53 PM, Martin Kletzander  wrote:

On Thu, Oct 20, 2016 at 11:26:09AM +0200, Ruben Kerkhof wrote:


Hi all,

virsh(1) has this to say about virsh shutdown:
"The exact behavior of a domain when it shuts down is set by the
on_shutdown parameter in the domain's XML definition."

on_shutdown isn't documented in https://libvirt.org/formatdomain.html


Is the virsh(1) manpage wrong?



Yes, it most certainly is.  The element is called  [1]


Thanks for your reply Martin.
Interestingly, I checked some of my xml, and they had 
in there next to 
These vms have been running for years and I might have even migrated
them from Xen's xm configs to libvirt / kvm at one point.

virsh dumpxml confirms that this element is just ignored, so no harm done.



Or have you possibly edited the on-disk saved state of libvirt some time
back instead of using virsh edit? =)



Patches welcome O:-)


Will do :)



Great!



Martin

[1] https://libvirt.org/formatdomain.html#elementsEvents


Kind regards,

Ruben

--
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 0/5] wireshark: Fix $(prefix) handling

[Michal Privoznik]

On 26.10.2016 15:27, Andrea Bolognani wrote:
> Well, almost :) There are still some cases that are not
> handled correctly, but at least this will unbreak 'make
> rpm' while I work on the rest.
> 
> Tested by running 'make rpm' successfully on Fedora 23,
> Fedora 24 and Fedora rawhide.
> 
> 
> Andrea Bolognani (5):
>   wireshark: Introduce $ws_modversion
>   wireshark: Hoist $ws_prefix declaration
>   wireshark: Strip prefix correctly
>   wireshark: Inject $(prefix) at the right time
>   wireshark: Rename plugindir to ws_plugindir
> 
>  m4/virt-wireshark.m4 | 29 ++---
>  tools/Makefile.am|  2 +-
>  2 files changed, 19 insertions(+), 12 deletions(-)
> 

ACK series. Thanks for cleaning up my mess.

Michal

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

Re: [libvirt] [PATCH] Resuming running domain raise error message

[Martin Kletzander]

On Sat, Oct 22, 2016 at 12:30:01PM +0200, Sławek Kapłoński wrote:

When user tries to resume already running domain (Qemu or LXC)
there is VIR_ERR_OPERATION_INVALID error raised now with
message that domain is already running.

Closes-Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1009008
---
src/lxc/lxc_driver.c   | 8 +++-
src/qemu/qemu_driver.c | 4 
2 files changed, 11 insertions(+), 1 deletion(-)



I wanted to split this into two commits, separate it for qemu and lxc,
but it's so small that when someone were to use this patch, it will fix
both qemu and lxc and most likely not cause any merge conflicts =)

Congratulations on your first patch.  I'll slightly reword the commit
message and push this.

Martin


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

Re: [libvirt] on shutdown

[Ruben Kerkhof]

On Thu, Oct 20, 2016 at 7:53 PM, Martin Kletzander  wrote:
> On Thu, Oct 20, 2016 at 11:26:09AM +0200, Ruben Kerkhof wrote:
>>
>> Hi all,
>>
>> virsh(1) has this to say about virsh shutdown:
>> "The exact behavior of a domain when it shuts down is set by the
>> on_shutdown parameter in the domain's XML definition."
>>
>> on_shutdown isn't documented in https://libvirt.org/formatdomain.html
>>
>>
>> Is the virsh(1) manpage wrong?
>>
>
> Yes, it most certainly is.  The element is called  [1]

Thanks for your reply Martin.
Interestingly, I checked some of my xml, and they had 
in there next to 
These vms have been running for years and I might have even migrated
them from Xen's xm configs to libvirt / kvm at one point.

virsh dumpxml confirms that this element is just ignored, so no harm done.

>
> Patches welcome O:-)

Will do :)

>
> Martin
>
> [1] https://libvirt.org/formatdomain.html#elementsEvents

Kind regards,

Ruben

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

Re: [libvirt] [PATCH] Resuming running domain raise error message

[Sławek Kapłoński]

Hello,

Can someone take a look at this patch? Thx in advance :)

-- 
Best regards / Pozdrawiam
Sławek Kapłoński
sla...@kaplonski.pl

On Sat, 22 Oct 2016, Sławek Kapłoński wrote:

> When user tries to resume already running domain (Qemu or LXC)
> there is VIR_ERR_OPERATION_INVALID error raised now with
> message that domain is already running.
> 
> Closes-Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1009008
> ---
>  src/lxc/lxc_driver.c   | 8 +++-
>  src/qemu/qemu_driver.c | 4 
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index 466e67f..4a0165a 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -3156,6 +3156,7 @@ static int lxcDomainResume(virDomainPtr dom)
>  virDomainObjPtr vm;
>  virObjectEventPtr event = NULL;
>  int ret = -1;
> +int state;
>  virLXCDomainObjPrivatePtr priv;
>  virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver);
>  
> @@ -3176,7 +3177,12 @@ static int lxcDomainResume(virDomainPtr dom)
>  goto endjob;
>  }
>  
> -if (virDomainObjGetState(vm, NULL) == VIR_DOMAIN_PAUSED) {
> +state = virDomainObjGetState(vm, NULL);
> +if (state == VIR_DOMAIN_RUNNING) {
> +virReportError(VIR_ERR_OPERATION_INVALID,
> +   "%s", _("domain is already running"));
> +goto endjob;
> +} else if (state == VIR_DOMAIN_PAUSED) {
>  if (virCgroupSetFreezerState(priv->cgroup, "THAWED") < 0) {
>  virReportError(VIR_ERR_OPERATION_FAILED,
> "%s", _("Resume operation failed"));
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 93ea5e2..c99186a 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -1919,6 +1919,10 @@ static int qemuDomainResume(virDomainPtr dom)
>  virReportError(VIR_ERR_OPERATION_INVALID,
> "%s", _("domain is pmsuspended"));
>  goto endjob;
> +} else if (state == VIR_DOMAIN_RUNNING) {
> +virReportError(VIR_ERR_OPERATION_INVALID,
> +   "%s", _("domain is already running"));
> +goto endjob;
>  } else if ((state == VIR_DOMAIN_CRASHED &&
>  reason == VIR_DOMAIN_CRASHED_PANICKED) ||
> state == VIR_DOMAIN_PAUSED) {
> -- 
> 2.10.1
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list


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

Re: [libvirt] [PATCH] virt-wireshark.m4: Defer $(prefix) substitution

[Andrea Bolognani]

On Wed, 2016-10-26 at 08:46 +0200, Boris Fiuczynski wrote:
> > >  if WITH_WIRESHARK_DISSECTOR
> > > 
> > > -ws_plugindir = $(plugindir)
> > > +ws_plugindir = $(prefix)$(plugindir)
> > >  ws_plugin_LTLIBRARIES = wireshark/src/libvirt.la
> > >  wireshark_src_libvirt_la_CPPFLAGS = \
> > >   -I wireshark/src $(WIRESHARK_DISSECTOR_CFLAGS)
> > 
> > It looks like this patch has broken the RPM build
> > 
> > https://ci.centos.org/view/libvirt-project/job/libvirt-master-rpm/82/systems=libvirt-fedora-23/console
> > 
> > Notice at the make install phase:
> > 
> >  /usr/bin/mkdir -p 
> >'/home/jenkins/rpmbuild/BUILDROOT/libvirt-2.4.0-1.fc23.x86_64/usr/usr/lib64/wireshark/plugins/1.12.12'
> >  /bin/sh ../libtool   --mode=install /usr/bin/install -c   
> >wireshark/src/libvirt.la '/home/jenkins/rpmbuild/BUILDROOT/libvirt-2.4.0-
1.fc23.x86_64/usr/usr/lib64/wireshark/plugins/1.12.12'
> > libtool: install: /usr/bin/install -c wireshark/src/.libs/libvirt.so 
> > /home/jenkins/rpmbuild/BUILDROOT/libvirt-2.4.0-
1.fc23.x86_64/usr/usr/lib64/wireshark/plugins/1.12.12/libvirt.so
> > libtool: install: /usr/bin/install -c wireshark/src/.libs/libvirt.lai 
> > /home/jenkins/rpmbuild/BUILDROOT/libvirt-2.4.0-
1.fc23.x86_64/usr/usr/lib64/wireshark/plugins/1.12.12/libvirt.la
> > libtool: warning: remember to run 'libtool --finish 
> > /usr/usr/lib64/wireshark/plugins/1.12.12'
> > 
> > 
> > It is getting "/usr/usr" in the path which is very wrong.
> 
> I can confirm that the rpm build is broken with wireshark versions < 2.
> The package config of these versions do not provide plugindir and the 
> code change in this patch is only working correctly when it is provided 
> and otherwise ends up with the scenario Daniel outlined above.
> 
> I think that line
> http://libvirt.org/git/?p=libvirt.git;a=blob;f=m4/virt-wireshark.m4;h=e1e4a598d627899791832455c8619af72a88f575;hb=HEAD#l35
> needs fixing but I have no good idea how to make the adjustment 
> compatible with the changes in the else branch.

I have a partial fix for this

  https://www.redhat.com/archives/libvir-list/2016-October/msg01169.html

Doesn't handle all cases yet but should be good enough to
make the CI job green again. Feel free to give it a go :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

[libvirt] [PATCH 0/5] wireshark: Fix $(prefix) handling

[Andrea Bolognani]

Well, almost :) There are still some cases that are not
handled correctly, but at least this will unbreak 'make
rpm' while I work on the rest.

Tested by running 'make rpm' successfully on Fedora 23,
Fedora 24 and Fedora rawhide.


Andrea Bolognani (5):
  wireshark: Introduce $ws_modversion
  wireshark: Hoist $ws_prefix declaration
  wireshark: Strip prefix correctly
  wireshark: Inject $(prefix) at the right time
  wireshark: Rename plugindir to ws_plugindir

 m4/virt-wireshark.m4 | 29 ++---
 tools/Makefile.am|  2 +-
 2 files changed, 19 insertions(+), 12 deletions(-)

-- 
2.7.4

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

[libvirt] [PATCH 4/5] wireshark: Inject $(prefix) at the right time

[Andrea Bolognani]

Adding $(prefix) in Makefile.am, as we were doing, means that
it would be prepended even when using --with-ws-plugindir,
which is something we don't want to happen.

Instead, we add it beforehand but take care that it doesn't
get expanded until make is called.
---
 m4/virt-wireshark.m4 | 6 +-
 tools/Makefile.am| 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/m4/virt-wireshark.m4 b/m4/virt-wireshark.m4
index 90d731e..c949f8a 100644
--- a/m4/virt-wireshark.m4
+++ b/m4/virt-wireshark.m4
@@ -42,7 +42,11 @@ AC_DEFUN([LIBVIRT_CHECK_WIRESHARK],[
 dnl /usr is our best bet
 ws_prefix="/usr"
   fi
-  plugindir="${plugindir#$ws_prefix}"
+  dnl Replace the wireshark prefix with our own.
+  dnl Note that $(prefix) is kept verbatim at this point in time, and will
+  dnl only be expanded later, when make is called: this makes it possible
+  dnl to override the prefix at compilation or installation time
+  plugindir='$(prefix)'"${plugindir#$ws_prefix}"
 elif test "x$with_ws_plugindir" = "xno" || test "x$with_ws_plugindir" = 
"xyes"; then
   AC_MSG_ERROR([ws-plugindir must be used only with valid path])
 else
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 08e1680..981be31 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -398,7 +398,7 @@ EXTRA_DIST += \
 
 if WITH_WIRESHARK_DISSECTOR
 
-ws_plugindir = $(prefix)$(plugindir)
+ws_plugindir = @plugindir@
 ws_plugin_LTLIBRARIES = wireshark/src/libvirt.la
 wireshark_src_libvirt_la_CPPFLAGS = \
-I wireshark/src $(WIRESHARK_DISSECTOR_CFLAGS)
-- 
2.7.4

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

[libvirt] [PATCH 5/5] wireshark: Rename plugindir to ws_plugindir

[Andrea Bolognani]

Since we're using autoconf to substitute the right value in
Makefile.am now, we can use a less generic name without running
into circular dependencies.
---
 m4/virt-wireshark.m4 | 12 ++--
 tools/Makefile.am|  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/m4/virt-wireshark.m4 b/m4/virt-wireshark.m4
index c949f8a..556272a 100644
--- a/m4/virt-wireshark.m4
+++ b/m4/virt-wireshark.m4
@@ -28,13 +28,13 @@ AC_DEFUN([LIBVIRT_CHECK_WIRESHARK],[
   dnl Check for system location of wireshark plugins
   if test "x$with_wireshark_dissector" != "xno" ; then
 if test "x$with_ws_plugindir" = "xcheck" ; then
-  plugindir="$($PKG_CONFIG --variable plugindir wireshark)"
+  ws_plugindir="$($PKG_CONFIG --variable plugindir wireshark)"
   ws_prefix="$($PKG_CONFIG --variable prefix wireshark)"
   ws_modversion="$($PKG_CONFIG --modversion wireshark)"
-  if test "x$plugindir" = "x" ; then
+  if test "x$ws_plugindir" = "x" ; then
 dnl On some systems the plugindir variable may not be stored within 
pkg config.
 dnl Fall back to older style of constructing the plugin dir path.
-plugindir="$libdir/wireshark/plugins/$ws_modversion"
+ws_plugindir="$libdir/wireshark/plugins/$ws_modversion"
 ws_prefix="$prefix"
   fi
   if test "x$ws_prefix" = "x" ; then
@@ -46,15 +46,15 @@ AC_DEFUN([LIBVIRT_CHECK_WIRESHARK],[
   dnl Note that $(prefix) is kept verbatim at this point in time, and will
   dnl only be expanded later, when make is called: this makes it possible
   dnl to override the prefix at compilation or installation time
-  plugindir='$(prefix)'"${plugindir#$ws_prefix}"
+  ws_plugindir='$(prefix)'"${ws_plugindir#$ws_prefix}"
 elif test "x$with_ws_plugindir" = "xno" || test "x$with_ws_plugindir" = 
"xyes"; then
   AC_MSG_ERROR([ws-plugindir must be used only with valid path])
 else
-  plugindir=$with_ws_plugindir
+  ws_plugindir=$with_ws_plugindir
 fi
   fi
 
-  AC_SUBST([plugindir])
+  AC_SUBST([ws_plugindir])
 ])
 
 AC_DEFUN([LIBVIRT_RESULT_WIRESHARK],[
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 981be31..319abb2 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -398,7 +398,7 @@ EXTRA_DIST += \
 
 if WITH_WIRESHARK_DISSECTOR
 
-ws_plugindir = @plugindir@
+ws_plugindir = @ws_plugindir@
 ws_plugin_LTLIBRARIES = wireshark/src/libvirt.la
 wireshark_src_libvirt_la_CPPFLAGS = \
-I wireshark/src $(WIRESHARK_DISSECTOR_CFLAGS)
-- 
2.7.4

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

[libvirt] [PATCH 3/5] wireshark: Strip prefix correctly

[Andrea Bolognani]

Even when we're building $plugindir ourselves because we can't
retrieve it using pkg-config, we still want to strip the prefix,
except in that case it would be the same prefix we're using for
building libvirt.

The fact that $plugindir is missing also doesn't tell us
anything about $ws_prefix, so we have to handle the two variables
separately.
---
 m4/virt-wireshark.m4 | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/m4/virt-wireshark.m4 b/m4/virt-wireshark.m4
index 64acca9..90d731e 100644
--- a/m4/virt-wireshark.m4
+++ b/m4/virt-wireshark.m4
@@ -35,12 +35,14 @@ AC_DEFUN([LIBVIRT_CHECK_WIRESHARK],[
 dnl On some systems the plugindir variable may not be stored within 
pkg config.
 dnl Fall back to older style of constructing the plugin dir path.
 plugindir="$libdir/wireshark/plugins/$ws_modversion"
-  else
-if test "x$ws_prefix" = "x" ; then
-  ws_prefix="/usr";
-fi
-plugindir="${plugindir#$ws_prefix}"
+ws_prefix="$prefix"
   fi
+  if test "x$ws_prefix" = "x" ; then
+dnl If the wireshark prefix cannot be retrieved from pkg-config,
+dnl /usr is our best bet
+ws_prefix="/usr"
+  fi
+  plugindir="${plugindir#$ws_prefix}"
 elif test "x$with_ws_plugindir" = "xno" || test "x$with_ws_plugindir" = 
"xyes"; then
   AC_MSG_ERROR([ws-plugindir must be used only with valid path])
 else
-- 
2.7.4

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

[libvirt] [PATCH 2/5] wireshark: Hoist $ws_prefix declaration

[Andrea Bolognani]

Keep all variable declarations close together.
---
 m4/virt-wireshark.m4 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/m4/virt-wireshark.m4 b/m4/virt-wireshark.m4
index eb6c8a6..64acca9 100644
--- a/m4/virt-wireshark.m4
+++ b/m4/virt-wireshark.m4
@@ -29,13 +29,13 @@ AC_DEFUN([LIBVIRT_CHECK_WIRESHARK],[
   if test "x$with_wireshark_dissector" != "xno" ; then
 if test "x$with_ws_plugindir" = "xcheck" ; then
   plugindir="$($PKG_CONFIG --variable plugindir wireshark)"
+  ws_prefix="$($PKG_CONFIG --variable prefix wireshark)"
   ws_modversion="$($PKG_CONFIG --modversion wireshark)"
   if test "x$plugindir" = "x" ; then
 dnl On some systems the plugindir variable may not be stored within 
pkg config.
 dnl Fall back to older style of constructing the plugin dir path.
 plugindir="$libdir/wireshark/plugins/$ws_modversion"
   else
-ws_prefix="$($PKG_CONFIG --variable prefix wireshark)"
 if test "x$ws_prefix" = "x" ; then
   ws_prefix="/usr";
 fi
-- 
2.7.4

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

[libvirt] [PATCH 1/5] wireshark: Introduce $ws_modversion

[Andrea Bolognani]

Use a separate variable instead of setting it inline for
slightly cleaner code.
---
 m4/virt-wireshark.m4 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/m4/virt-wireshark.m4 b/m4/virt-wireshark.m4
index e1e4a59..eb6c8a6 100644
--- a/m4/virt-wireshark.m4
+++ b/m4/virt-wireshark.m4
@@ -29,10 +29,11 @@ AC_DEFUN([LIBVIRT_CHECK_WIRESHARK],[
   if test "x$with_wireshark_dissector" != "xno" ; then
 if test "x$with_ws_plugindir" = "xcheck" ; then
   plugindir="$($PKG_CONFIG --variable plugindir wireshark)"
+  ws_modversion="$($PKG_CONFIG --modversion wireshark)"
   if test "x$plugindir" = "x" ; then
 dnl On some systems the plugindir variable may not be stored within 
pkg config.
 dnl Fall back to older style of constructing the plugin dir path.
-plugindir="$libdir/wireshark/plugins/$($PKG_CONFIG --modversion 
wireshark)"
+plugindir="$libdir/wireshark/plugins/$ws_modversion"
   else
 ws_prefix="$($PKG_CONFIG --variable prefix wireshark)"
 if test "x$ws_prefix" = "x" ; then
-- 
2.7.4

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

[libvirt] [PATCH 13/17] qemu.conf: Introduce write_udev

[Michal Privoznik]

Not everybody is going to use the new virUdevMgr module. Some
users have their own set of udev rules and they don't need
libvirt to step in that area. Lets give users choice to enable or
disable this feature.

Signed-off-by: Michal Privoznik 
---
 src/qemu/libvirtd_qemu.aug | 1 +
 src/qemu/qemu.conf | 5 +
 src/qemu/qemu_conf.c   | 3 +++
 src/qemu/qemu_conf.h   | 5 +
 src/qemu/qemu_driver.c | 8 
 src/qemu/test_libvirtd_qemu.aug.in | 1 +
 6 files changed, 23 insertions(+)

diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index 73ebeda..08e0803 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -70,6 +70,7 @@ module Libvirtd_qemu =
  | str_array_entry "cgroup_controllers"
  | str_array_entry "cgroup_device_acl"
  | int_entry "seccomp_sandbox"
+ | bool_entry "write_udev"
 
let save_entry =  str_entry "save_image_format"
  | str_entry "dump_image_format"
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index c4fcb6d..a34975a 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -293,6 +293,11 @@
 # guests will be blocked. Defaults to 0.
 #security_require_confined = 1
 
+# In order to avoid races between libvirt and udev who also changes security
+# labels on devices, libvirt can let know what devices belong a domain managed
+# by libvirt and thus reason udev to not mangle security labels.
+#write_udev = 1
+
 # The user for QEMU processes run by the system instance. It can be
 # specified as a user name or as a user id. The qemu driver will try to
 # parse this value first as a name and then, if the name doesn't exist,
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 109668b..c0be670 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -795,6 +795,9 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
 }
 }
 
+if (virConfGetValueBool(conf, "write_udev", &cfg->writeUdev) < 0)
+goto cleanup;
+
 ret = 0;
 
  cleanup:
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 12b2661..b42eea7 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -192,6 +192,8 @@ struct _virQEMUDriverConfig {
 
 virFirmwarePtr *firmwares;
 size_t nfirmwares;
+
+bool writeUdev;
 };
 
 /* Main driver state */
@@ -269,6 +271,9 @@ struct _virQEMUDriver {
 
 /* Immutable pointer, self-locking APIs */
 virHashAtomicPtr migrationErrors;
+
+/* Immutable pointer, self-locking APIs*/
+virUdevMgrPtr udevMgr;
 };
 
 typedef struct _qemuDomainCmdlineDef qemuDomainCmdlineDef;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3a51826..7dbbc25 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -378,6 +378,10 @@ qemuSecurityInit(virQEMUDriverPtr driver)
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 unsigned int flags = 0;
 
+if (cfg->writeUdev &&
+!(driver->udevMgr = virUdevMgrNew()))
+goto error;
+
 if (cfg->allowDiskFormatProbing)
 flags |= VIR_SECURITY_MANAGER_ALLOW_DISK_PROBE;
 if (cfg->securityDefaultConfined)
@@ -395,6 +399,7 @@ qemuSecurityInit(virQEMUDriverPtr driver)
   QEMU_DRIVER_NAME,
   flags)))
 goto error;
+virSecurityManagerSetUdevManager(mgr, driver->udevMgr);
 if (!stack) {
 if (!(stack = virSecurityManagerNewStack(mgr)))
 goto error;
@@ -410,6 +415,7 @@ qemuSecurityInit(virQEMUDriverPtr driver)
   QEMU_DRIVER_NAME,
   flags)))
 goto error;
+virSecurityManagerSetUdevManager(mgr, driver->udevMgr);
 if (!(stack = virSecurityManagerNewStack(mgr)))
 goto error;
 mgr = NULL;
@@ -424,6 +430,7 @@ qemuSecurityInit(virQEMUDriverPtr driver)
  flags,
  qemuSecurityChownCallback)))
 goto error;
+virSecurityManagerSetUdevManager(mgr, driver->udevMgr);
 if (!stack) {
 if (!(stack = virSecurityManagerNewStack(mgr)))
 goto error;
@@ -1091,6 +1098,7 @@ qemuStateCleanup(void)
 VIR_FREE(qemu_driver->qemuImgBinary);
 
 virObjectUnref(qemu_driver->securityManager);
+virObjectUnref(qemu_driver->udevMgr);
 
 ebtablesContextFree(qemu_driver->ebtables);
 
diff --git a/src/qemu/test_libvirtd_qemu.aug.in 
b/src/qemu/test_libvirtd_qemu.aug.in
index 805fa0e..112f343 100644
--- a/src/qemu/test_libvirtd_qemu.aug.in
+++ b/src/qemu/test_libvirtd_qemu.aug.in
@@ -33,6 +33,7 @@ module Test_libvirtd_qemu =
 { "security_driver" = "selinux" }
 { "security_default_confined" = "1" }
 { "security_require_confined" = "1" }

[libvirt] [PATCH 14/17] qemu: Wire up virUdevMgr

[Michal Privoznik]

Now that security drivers are capable of writing into virUdevMgr
module, we also need it to flush its internal database right
after that.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_domain.c  | 12 +++-
 src/qemu/qemu_domain.h  |  3 ++-
 src/qemu/qemu_driver.c  |  9 +++--
 src/qemu/qemu_hotplug.c | 35 ---
 src/qemu/qemu_process.c | 47 +--
 src/qemu/qemu_process.h |  3 +++
 6 files changed, 96 insertions(+), 13 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 838e838..4422fbc 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -641,13 +641,15 @@ qemuDomainMasterKeyReadFile(qemuDomainObjPrivatePtr priv)
 
 
 /* qemuDomainMasterKeyRemove:
+ * @driver: qemu driver data
  * @priv: Pointer to the domain private object
  *
  * Remove the traces of the master key, clear the heap, clear the file,
  * delete the file.
  */
 void
-qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv)
+qemuDomainMasterKeyRemove(virQEMUDriverPtr driver,
+  qemuDomainObjPrivatePtr priv)
 {
 char *path = NULL;
 
@@ -660,6 +662,8 @@ qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv)
 /* Delete the master key file */
 path = qemuDomainGetMasterKeyFilePath(priv->libDir);
 unlink(path);
+if (driver->udevMgr)
+virUdevMgrRemoveAllLabels(driver->udevMgr, path);
 
 VIR_FREE(path);
 }
@@ -4935,6 +4939,9 @@ qemuDomainDiskChainElementRevoke(virQEMUDriverPtr driver,
 vm->def, elem) < 0)
 VIR_WARN("Unable to restore security label on %s", 
NULLSTR(elem->path));
 
+if (qemuProcessFlushUdev(driver) < 0)
+VIR_WARN("Unable to clean up udev rules");
+
 if (qemuTeardownImageCgroup(vm, elem) < 0)
 VIR_WARN("Failed to teardown cgroup for disk path %s",
  NULLSTR(elem->path));
@@ -4974,6 +4981,9 @@ qemuDomainDiskChainElementPrepare(virQEMUDriverPtr driver,
 elem) < 0)
 goto cleanup;
 
+if (qemuProcessFlushUdev(driver) < 0)
+goto cleanup;
+
 ret = 0;
 
  cleanup:
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 2ee1829..a381b59 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -710,7 +710,8 @@ int qemuDomainWriteMasterKeyFile(virQEMUDriverPtr driver,
 
 int qemuDomainMasterKeyCreate(virDomainObjPtr vm);
 
-void qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv);
+void qemuDomainMasterKeyRemove(virQEMUDriverPtr driver,
+   qemuDomainObjPrivatePtr priv);
 
 void qemuDomainSecretDiskDestroy(virDomainDiskDefPtr disk)
 ATTRIBUTE_NONNULL(1);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7dbbc25..cc34782 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3864,8 +3864,11 @@ qemuDomainScreenshot(virDomainPtr dom,
 
  endjob:
 VIR_FORCE_CLOSE(tmp_fd);
-if (unlink_tmp)
+if (unlink_tmp) {
+virSecurityManagerRestoreSavedStateLabel(driver->securityManager, 
vm->def, tmp);
+qemuProcessFlushUdev(driver);
 unlink(tmp);
+}
 VIR_FREE(tmp);
 
 qemuDomainObjEndJob(driver, vm);
@@ -6682,6 +6685,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
 if (virSecurityManagerRestoreSavedStateLabel(driver->securityManager,
  vm->def, path) < 0)
 VIR_WARN("failed to restore save state label on %s", path);
+qemuProcessFlushUdev(driver);
 virObjectUnref(cfg);
 return ret;
 }
@@ -16071,7 +16075,8 @@ qemuDomainBlockPivot(virQEMUDriverPtr driver,
  disk) < 0 ||
  qemuSetupDiskCgroup(vm, disk) < 0 ||
  virSecurityManagerSetDiskLabel(driver->securityManager, vm->def,
-disk) < 0))
+disk) < 0 ||
+ qemuProcessFlushUdev(driver) < 0))
 goto cleanup;
 
 disk->src = oldsrc;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 9746a06..fdac464 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -113,6 +113,9 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver,
vm->def, disk) < 0)
 goto rollback_lock;
 
+if (qemuProcessFlushUdev(driver) < 0)
+goto rollback_label;
+
 if (qemuSetupDiskCgroup(vm, disk) < 0)
 goto rollback_label;
 
@@ -130,6 +133,9 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver,
 VIR_WARN("Unable to restore security label on %s",
  virDomainDiskGetSource(disk));
 
+if (qemuProcessFlushUdev(driver) < 0)
+VIR_WARN("Unable to clean up udev rules");
+
  rollback_lock:
 if (virDomainLockDiskDetach(driver->lockManager, vm, disk) < 0)
 VIR_WARN("Unable to release lock on %s",
@@ -1427,6 +14

[libvirt] [PATCH 08/17] tests: Introduce virudevtest

[Michal Privoznik]

Now that we have the virudev module somewhat working, lets
introduce some testing of it.

We also need mock for virRandomBits function. Without it, the
order in which entries occur in the hash table would be random
and thus test would fail some times (as we expect certain
ordering).

Signed-off-by: Michal Privoznik 
---
 tests/Makefile.am |  12 +++
 tests/virudevmock.c   |  29 +++
 tests/virudevtest.c   | 124 ++
 tests/virudevtestdata/complex.json|  30 
 tests/virudevtestdata/empty.json  |   5 ++
 tests/virudevtestdata/simple-dac.json |  13 
 tests/virudevtestdata/simple-selinux.json |  13 
 7 files changed, 226 insertions(+)
 create mode 100644 tests/virudevmock.c
 create mode 100644 tests/virudevtest.c
 create mode 100644 tests/virudevtestdata/complex.json
 create mode 100644 tests/virudevtestdata/empty.json
 create mode 100644 tests/virudevtestdata/simple-dac.json
 create mode 100644 tests/virudevtestdata/simple-selinux.json

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 924029a..824796b 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -150,6 +150,7 @@ EXTRA_DIST =\
virpcitestdata \
virscsidata \
virsh-uriprecedence \
+   virudevtestdata \
virusbtestdata \
vmwareverdata \
vmx2xmldata \
@@ -193,6 +194,7 @@ test_programs = virshtest sockettest \
vircaps2xmltest \
virnetdevtest \
virtypedparamtest \
+   virudevtest \
$(NULL)
 
 if WITH_REMOTE
@@ -406,6 +408,7 @@ test_libraries = libshunload.la \
virhostcpumock.la \
nssmock.la \
domaincapsmock.la \
+   virudevmock.la \
$(NULL)
 if WITH_QEMU
 test_libraries += libqemumonitortestutils.la \
@@ -1343,6 +1346,15 @@ virtypedparamtest_SOURCES = \
virtypedparamtest.c testutils.h testutils.c
 virtypedparamtest_LDADD = $(LDADDS)
 
+virudevtest_SOURCES = \
+   virudevtest.c testutils.h testutils.c
+virudevtest_LDADD = $(LDADDS)
+
+virudevmock_la_SOURCES = virudevmock.c
+virudevmock_la_CFLAGS = $(AM_CFLAGS)
+virudevmock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS)
+virudevmock_la_LIBADD = $(MOCKLIBS_LIBS)
+
 
 if WITH_LINUX
 fchosttest_SOURCES = \
diff --git a/tests/virudevmock.c b/tests/virudevmock.c
new file mode 100644
index 000..d01f1c9
--- /dev/null
+++ b/tests/virudevmock.c
@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2016 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ *
+ * Author: Michal Privoznik 
+ */
+
+#include 
+
+#include "virrandom.h"
+
+uint64_t virRandomBits(int nbits ATTRIBUTE_UNUSED)
+{
+return 4; /* chosen by fair dice roll.
+ guaranteed to be random. */
+}
diff --git a/tests/virudevtest.c b/tests/virudevtest.c
new file mode 100644
index 000..883d751
--- /dev/null
+++ b/tests/virudevtest.c
@@ -0,0 +1,124 @@
+/*
+ * Copyright (C) 2016 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ *
+ * Author: Michal Privoznik 
+ */
+
+#include 
+
+#include "testutils.h"
+#include "virudev.h"
+#include "virjson.h"
+
+#define VIR_FROM_THIS VIR_FROM_NONE
+
+struct testUdevData {
+const char *file;
+const char *const *labels;
+};
+
+
+static int
+testDump(const void *opaque)
+{
+const struct testUdevData *data = opaque;
+virUdevMgrPtr mgr = NULL;
+int ret = -1;
+const char * const *tmp;
+char *state = NULL;
+char *filename = NULL;
+
+if (virAsprintf(&filename, "%s/virudevtestdata/%s.json",
+abs_srcdir, data->file) < 0)
+goto cleanup;
+
+if (!(mgr = virUdevMgrNew())

[libvirt] [PATCH 01/17] virseclabel.h: Include stdbool.h

[Michal Privoznik]

The internal representation of security labels contains some
bools (e.g. @relabel, @implicit). But the stdbool.h file is not
included anywhere in the header file, therefore if somebody just
includes "virseclabel.h" they also need to include .
That's not right as it should be virseclabel.h who drags in all
the dependencies.

Signed-off-by: Michal Privoznik 
---
 src/util/virseclabel.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/util/virseclabel.h b/src/util/virseclabel.h
index 94c4dfc..558d4eb 100644
--- a/src/util/virseclabel.h
+++ b/src/util/virseclabel.h
@@ -22,6 +22,8 @@
 #ifndef __SECLABEL_H
 # define __SECLABEL_H
 
+# include 
+
 typedef enum {
 VIR_DOMAIN_SECLABEL_DEFAULT,
 VIR_DOMAIN_SECLABEL_NONE,
-- 
2.8.4

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

[libvirt] [PATCH 07/17] virudev: Introduce virUdevMgrDump

[Michal Privoznik]

Now that we are able to store security labels for devices, next
step is to flush them into a file. For more convenience I've
chosen JSON format (as we have all the APIs needed for processing
the format).

Signed-off-by: Michal Privoznik 
---
 po/POTFILES.in   |   1 +
 src/libvirt_private.syms |   2 +
 src/util/virudev.c   | 157 +++
 src/util/virudev.h   |   5 ++
 4 files changed, 165 insertions(+)

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 1469240..dabc612 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -248,6 +248,7 @@ src/util/virthreadpool.c
 src/util/virtime.c
 src/util/virtpm.c
 src/util/virtypedparam.c
+src/util/virudev.c
 src/util/viruri.c
 src/util/virusb.c
 src/util/virutil.c
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 184317a..b465a0d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2575,6 +2575,8 @@ virTypedParamsValidate;
 
 # util/virudev.h
 virUdevMgrAddLabel;
+virUdevMgrDumpFile;
+virUdevMgrDumpStr;
 virUdevMgrNew;
 virUdevMgrRemoveAllLabels;
 
diff --git a/src/util/virudev.c b/src/util/virudev.c
index f4799e7..7f52149 100644
--- a/src/util/virudev.c
+++ b/src/util/virudev.c
@@ -24,7 +24,9 @@
 
 #include "internal.h"
 #include "viralloc.h"
+#include "virfile.h"
 #include "virhash.h"
+#include "virjson.h"
 #include "virobject.h"
 #include "virudev.h"
 
@@ -112,6 +114,68 @@ udevSeclabelUpdate(udevSeclabelPtr list,
 }
 
 
+static virJSONValuePtr
+udevSeclabelDump(const virSecurityDeviceLabelDef *seclabel)
+{
+virJSONValuePtr object;
+
+if (!(object = virJSONValueNewObject()) ||
+virJSONValueObjectAppendString(object, "model", seclabel->model) < 0 ||
+virJSONValueObjectAppendString(object, "label", seclabel->label) < 0)
+goto error;
+
+return object;
+
+ error:
+virJSONValueFree(object);
+return NULL;
+}
+
+
+static int
+udevSeclabelsDump(void *payload,
+  const void *name,
+  void *opaque)
+{
+udevSeclabelPtr list = payload;
+const char *device = name;
+virJSONValuePtr seclabels = opaque;
+virJSONValuePtr deviceLabels = NULL;
+virJSONValuePtr deviceJSON = NULL;
+size_t i;
+int ret = -1;
+
+if (!(deviceLabels = virJSONValueNewArray()))
+return ret;
+
+for (i = 0; i < list->nseclabels; i++) {
+virJSONValuePtr seclabel = udevSeclabelDump(list->seclabels[i]);
+
+if (!seclabel ||
+virJSONValueArrayAppend(deviceLabels, seclabel) < 0) {
+virJSONValueFree(seclabel);
+goto cleanup;
+}
+}
+
+if (!(deviceJSON = virJSONValueNewObject()) ||
+virJSONValueObjectAppendString(deviceJSON, "device", device) < 0 ||
+virJSONValueObjectAppend(deviceJSON, "labels", deviceLabels) < 0)
+goto cleanup;
+deviceLabels = NULL;
+
+if (virJSONValueArrayAppend(seclabels, deviceJSON) < 0)
+goto cleanup;
+deviceJSON = NULL;
+
+ret = 0;
+ cleanup:
+virJSONValueFree(deviceJSON);
+virJSONValueFree(deviceLabels);
+return ret;
+}
+
+
 static void
 virUdevMgrDispose(void *obj)
 {
@@ -202,3 +266,96 @@ virUdevMgrRemoveAllLabels(virUdevMgrPtr mgr,
 virObjectUnlock(mgr);
 return ret;
 }
+
+
+static virJSONValuePtr
+virUdevSeclabelDump(virUdevMgrPtr mgr)
+{
+virJSONValuePtr seclabels;
+
+if (!(seclabels = virJSONValueNewArray()))
+return NULL;
+
+if (virHashForEach(mgr->labels, udevSeclabelsDump, seclabels) < 0) {
+virJSONValueFree(seclabels);
+return NULL;
+}
+
+return seclabels;
+}
+
+
+static char *
+virUdevMgrDumpInternal(virUdevMgrPtr mgr)
+{
+virJSONValuePtr object = NULL;
+virJSONValuePtr child = NULL;
+char *ret = NULL;
+
+if (!(object = virJSONValueNewObject()))
+goto cleanup;
+
+if (!(child = virUdevSeclabelDump(mgr)))
+goto cleanup;
+
+if (virJSONValueObjectAppend(object, "labels", child) < 0) {
+virJSONValueFree(child);
+goto cleanup;
+}
+
+ret = virJSONValueToString(object, true);
+ cleanup:
+virJSONValueFree(object);
+return ret;
+}
+
+
+char *
+virUdevMgrDumpStr(virUdevMgrPtr mgr)
+{
+char *ret;
+
+virObjectLock(mgr);
+ret = virUdevMgrDumpInternal(mgr);
+virObjectUnlock(mgr);
+return ret;
+}
+
+
+static int
+virUdevMgrRewriter(int fd, void *opaque)
+{
+const char *str = opaque;
+
+return safewrite(fd, str, strlen(str));
+}
+
+
+int
+virUdevMgrDumpFile(virUdevMgrPtr mgr,
+   const char *filename)
+{
+int ret = -1;
+char *state;
+
+virObjectLock(mgr);
+
+if (!(state = virUdevMgrDumpInternal(mgr)))
+goto cleanup;
+
+/* Here we shouldn't use pure virFileWriteStr() as that one is not atomic.
+ * We can be interrupted in the middle (e.g. due to a context switch) and
+ * thus leave the file partially written. */
+if (virFileRewrite(filename, 0644, virUdevMgrRewri

[libvirt] [PATCH 10/17] virudev: Introduce virUdevMgrLookupLabels

[Michal Privoznik]

This is the cherry on the top. For given device all its security
labels are fetched.

Signed-off-by: Michal Privoznik 
---
 src/libvirt_private.syms |  1 +
 src/util/virudev.c   | 43 +++
 src/util/virudev.h   |  4 +++
 tests/virudevtest.c  | 91 
 4 files changed, 139 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3369600..a0de4e9 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2577,6 +2577,7 @@ virTypedParamsValidate;
 virUdevMgrAddLabel;
 virUdevMgrDumpFile;
 virUdevMgrDumpStr;
+virUdevMgrLookupLabels;
 virUdevMgrNew;
 virUdevMgrNewFromFile;
 virUdevMgrNewFromStr;
diff --git a/src/util/virudev.c b/src/util/virudev.c
index d60fbf9..9940f5f 100644
--- a/src/util/virudev.c
+++ b/src/util/virudev.c
@@ -300,6 +300,49 @@ virUdevMgrAddLabel(virUdevMgrPtr mgr,
 
 
 int
+virUdevMgrLookupLabels(virUdevMgrPtr mgr,
+   const char *device,
+   virSecurityDeviceLabelDefPtr **seclabels,
+   size_t *nseclabels)
+{
+int ret = -1;
+udevSeclabelPtr list;
+size_t i;
+
+virObjectLock(mgr);
+
+if (!(list = virHashLookup(mgr->labels, device))) {
+*seclabels = NULL;
+*nseclabels = 0;
+ret = 0;
+goto cleanup;
+}
+
+if (VIR_ALLOC_N(*seclabels, list->nseclabels) < 0)
+goto cleanup;
+
+*nseclabels = list->nseclabels;
+
+for (i = 0; i < list->nseclabels; i++) {
+if (!((*seclabels)[i] = 
virSecurityDeviceLabelDefCopy(list->seclabels[i])))
+goto cleanup;
+}
+
+ret = 0;
+ cleanup:
+virObjectUnlock(mgr);
+if (ret < 0) {
+if (*seclabels) {
+for (i = 0; i < *nseclabels; i++)
+virSecurityDeviceLabelDefFree((*seclabels)[i]);
+VIR_FREE(*seclabels);
+}
+}
+return ret;
+}
+
+
+int
 virUdevMgrRemoveAllLabels(virUdevMgrPtr mgr,
   const char *device)
 {
diff --git a/src/util/virudev.h b/src/util/virudev.h
index 82b2b4f..4e286bb 100644
--- a/src/util/virudev.h
+++ b/src/util/virudev.h
@@ -37,6 +37,10 @@ int virUdevMgrAddLabel(virUdevMgrPtr mgr,
const virSecurityDeviceLabelDef *seclabel);
 int virUdevMgrRemoveAllLabels(virUdevMgrPtr mgr,
   const char *device);
+int virUdevMgrLookupLabels(virUdevMgrPtr mgr,
+   const char *device,
+   virSecurityDeviceLabelDefPtr **seclabels,
+   size_t *nseclabels);
 
 char *virUdevMgrDumpStr(virUdevMgrPtr mgr);
 
diff --git a/tests/virudevtest.c b/tests/virudevtest.c
index c395741..36a9077 100644
--- a/tests/virudevtest.c
+++ b/tests/virudevtest.c
@@ -124,6 +124,76 @@ testParse(const void *opaque)
 
 
 static int
+testLookup(const void *opaque)
+{
+const struct testUdevData *data = opaque;
+virUdevMgrPtr mgr = NULL;
+int ret = -1;
+const char * const *tmp;
+char *filename = NULL;
+virSecurityDeviceLabelDefPtr *seclabels = NULL;
+size_t i, nseclabels = 0;
+
+if (virAsprintf(&filename, "%s/virudevtestdata/%s.json",
+abs_srcdir, data->file) < 0)
+goto cleanup;
+
+if (!(mgr = virUdevMgrNewFromFile(filename)))
+goto cleanup;
+
+tmp = data->labels;
+while (*tmp) {
+const char *device;
+const char *model;
+const char *label;
+
+device = *tmp;
+if (!++tmp) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Invalid seclabels 
list");
+goto cleanup;
+}
+model = *tmp;
+if (!++tmp) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Invalid seclabels 
list");
+goto cleanup;
+}
+label = *tmp;
+tmp++;
+
+if (virUdevMgrLookupLabels(mgr, device, &seclabels, &nseclabels) < 0)
+goto cleanup;
+
+for (i = 0; i < nseclabels; i++) {
+virSecurityDeviceLabelDefPtr seclabel = seclabels[i];
+
+if (STREQ(seclabel->model, model) &&
+STREQ(seclabel->label, label))
+break;
+}
+
+if (i == nseclabels) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Label not found");
+goto cleanup;
+}
+
+for (i = 0; i < nseclabels; i++)
+virSecurityDeviceLabelDefFree(seclabels[i]);
+VIR_FREE(seclabels);
+nseclabels = 0;
+}
+
+ret = 0;
+ cleanup:
+for (i = 0; i < nseclabels; i++)
+virSecurityDeviceLabelDefFree(seclabels[i]);
+VIR_FREE(seclabels);
+VIR_FREE(filename);
+virObjectUnref(mgr);
+return ret;
+}
+
+
+static int
 mymain(void)
 {
 int ret = 0;
@@ -147,6 +217,16 @@ mymain(void)
 ret = -1;   \
 } while (0)
 
+#define DO_TEST_LOOKUP(filename, ...)   

[libvirt] [PATCH 03/17] security_dac: Pass manager to virSecurityDACSetImageLabel

[Michal Privoznik]

This change alone is not needed, but it prepares environment for
subsequent patches where we will need virSecurityManager much
deeper in the code.

Signed-off-by: Michal Privoznik 
---
 src/security/security_dac.c | 33 +
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index fd74e8b..97be862 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -349,12 +349,13 @@ virSecurityDACSetOwnershipInternal(virSecurityDACDataPtr 
priv,
 
 
 static int
-virSecurityDACSetOwnership(virSecurityDACDataPtr priv,
+virSecurityDACSetOwnership(virSecurityManagerPtr mgr,
virStorageSourcePtr src,
const char *path,
uid_t uid,
gid_t gid)
 {
+virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
 struct stat sb;
 
 if (!path && src && src->path &&
@@ -442,7 +443,7 @@ virSecurityDACSetImageLabel(virSecurityManagerPtr mgr,
 return -1;
 }
 
-return virSecurityDACSetOwnership(priv, src, NULL, user, group);
+return virSecurityDACSetOwnership(mgr, src, NULL, user, group);
 }
 
 
@@ -550,7 +551,7 @@ virSecurityDACSetHostdevLabelHelper(const char *file,
 if (virSecurityDACGetIds(secdef, priv, &user, &group, NULL, NULL) < 0)
 return -1;
 
-return virSecurityDACSetOwnership(priv, NULL, file, user, group);
+return virSecurityDACSetOwnership(mgr, NULL, file, user, group);
 }
 
 
@@ -850,7 +851,7 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
 switch ((virDomainChrType) dev_source->type) {
 case VIR_DOMAIN_CHR_TYPE_DEV:
 case VIR_DOMAIN_CHR_TYPE_FILE:
-ret = virSecurityDACSetOwnership(priv, NULL,
+ret = virSecurityDACSetOwnership(mgr, NULL,
  dev_source->data.file.path,
  user, group);
 break;
@@ -860,10 +861,10 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
 virAsprintf(&out, "%s.out", dev_source->data.file.path) < 0)
 goto done;
 if (virFileExists(in) && virFileExists(out)) {
-if (virSecurityDACSetOwnership(priv, NULL, in, user, group) < 0 ||
-virSecurityDACSetOwnership(priv, NULL, out, user, group) < 0)
+if (virSecurityDACSetOwnership(mgr, NULL, in, user, group) < 0 ||
+virSecurityDACSetOwnership(mgr, NULL, out, user, group) < 0)
 goto done;
-} else if (virSecurityDACSetOwnership(priv, NULL,
+} else if (virSecurityDACSetOwnership(mgr, NULL,
   dev_source->data.file.path,
   user, group) < 0) {
 goto done;
@@ -873,7 +874,7 @@ virSecurityDACSetChardevLabel(virSecurityManagerPtr mgr,
 
 case VIR_DOMAIN_CHR_TYPE_UNIX:
 if (!dev_source->data.nix.listen) {
-if (virSecurityDACSetOwnership(priv, NULL,
+if (virSecurityDACSetOwnership(mgr, NULL,
dev_source->data.nix.path,
user, group) < 0)
 goto done;
@@ -1033,7 +1034,7 @@ virSecurityDACSetInputLabel(virSecurityManagerPtr mgr,
 if (virSecurityDACGetIds(seclabel, priv, &user, &group, NULL, NULL) < 
0)
 return -1;
 
-ret = virSecurityDACSetOwnership(priv, NULL, input->source.evdev, 
user, group);
+ret = virSecurityDACSetOwnership(mgr, NULL, input->source.evdev, user, 
group);
 break;
 
 case VIR_DOMAIN_INPUT_TYPE_MOUSE:
@@ -1199,27 +1200,27 @@ virSecurityDACSetAllLabel(virSecurityManagerPtr mgr,
 return -1;
 
 if (def->os.loader && def->os.loader->nvram &&
-virSecurityDACSetOwnership(priv, NULL,
+virSecurityDACSetOwnership(mgr, NULL,
def->os.loader->nvram, user, group) < 0)
 return -1;
 
 if (def->os.kernel &&
-virSecurityDACSetOwnership(priv, NULL,
+virSecurityDACSetOwnership(mgr, NULL,
def->os.kernel, user, group) < 0)
 return -1;
 
 if (def->os.initrd &&
-virSecurityDACSetOwnership(priv, NULL,
+virSecurityDACSetOwnership(mgr, NULL,
def->os.initrd, user, group) < 0)
 return -1;
 
 if (def->os.dtb &&
-virSecurityDACSetOwnership(priv, NULL,
+virSecurityDACSetOwnership(mgr, NULL,
def->os.dtb, user, group) < 0)
 return -1;
 
 if (def->os.slic_table &&
-virSecurityDACSetOwnership(priv, NULL,
+virSecurityDACSetOwnership(mgr, NULL,
def->os.slic_table, user, group) < 0)
 return -1;
 
@@ -1242,7 +1243,7 @@ virSecurityDACSetSavedStateLabel(vir

[libvirt] [PATCH 12/17] security: Wire up virUdevMgr

[Michal Privoznik]

Whenever a security driver wants to change label of some path, it
should let virUdevMgr module know so that it can update its
internal database too.

Signed-off-by: Michal Privoznik 
---
 src/libvirt_private.syms|  2 ++
 src/security/security_dac.c | 36 ---
 src/security/security_manager.c | 16 ++
 src/security/security_manager.h |  5 +
 src/security/security_selinux.c | 47 ++---
 5 files changed, 100 insertions(+), 6 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a0de4e9..66df1f7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1143,6 +1143,7 @@ virSecurityManagerGetModel;
 virSecurityManagerGetMountOptions;
 virSecurityManagerGetNested;
 virSecurityManagerGetProcessLabel;
+virSecurityManagerGetUdevManager;
 virSecurityManagerNew;
 virSecurityManagerNewDAC;
 virSecurityManagerNewStack;
@@ -1167,6 +1168,7 @@ virSecurityManagerSetProcessLabel;
 virSecurityManagerSetSavedStateLabel;
 virSecurityManagerSetSocketLabel;
 virSecurityManagerSetTapFDLabel;
+virSecurityManagerSetUdevManager;
 virSecurityManagerStackAddNested;
 virSecurityManagerVerify;
 
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 7f17124..54e59c7 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -356,7 +356,11 @@ virSecurityDACSetOwnership(virSecurityManagerPtr mgr,
gid_t gid)
 {
 virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+virUdevMgrPtr udevMgr = virSecurityManagerGetUdevManager(mgr);
+virSecurityDeviceLabelDefPtr seclabel = NULL;
+char *label = NULL;
 struct stat sb;
+int ret = -1;
 
 if (!path && src && src->path &&
 virStorageSourceIsLocalStorage(src))
@@ -365,14 +369,36 @@ virSecurityDACSetOwnership(virSecurityManagerPtr mgr,
 if (path) {
 if (stat(path, &sb) < 0) {
 virReportSystemError(errno, _("unable to stat: %s"), path);
-return -1;
+return ret;
 }
 
 if (virSecurityDACRememberLabel(priv, path, sb.st_uid, sb.st_gid) < 0)
-return -1;
+return ret;
+
+if (udevMgr) {
+if (virAsprintf(&label, "%u %u",
+(unsigned int) uid,
+(unsigned int) gid) < 0)
+goto cleanup;
+
+if (!(seclabel = 
virSecurityDeviceLabelDefNewLabel(SECURITY_DAC_NAME, label)))
+goto cleanup;
+VIR_FREE(label);
+}
 }
 
-return virSecurityDACSetOwnershipInternal(priv, src, path, uid, gid);
+if (virSecurityDACSetOwnershipInternal(priv, src, path, uid, gid) < 0)
+goto cleanup;
+
+if (udevMgr && path &&
+virUdevMgrAddLabel(udevMgr, path, seclabel) < 0)
+goto cleanup;
+
+ret = 0;
+ cleanup:
+VIR_FREE(label);
+virSecurityDeviceLabelDefFree(seclabel);
+return ret;
 }
 
 
@@ -382,6 +408,7 @@ 
virSecurityDACRestoreFileLabelInternal(virSecurityManagerPtr mgr,
const char *path)
 {
 virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+virUdevMgrPtr udevMgr = virSecurityManagerGetUdevManager(mgr);
 int rv;
 uid_t uid = 0;  /* By default return to root:root */
 gid_t gid = 0;
@@ -399,6 +426,9 @@ 
virSecurityDACRestoreFileLabelInternal(virSecurityManagerPtr mgr,
 return -1;
 if (rv > 0)
 return 0;
+
+if (udevMgr)
+virUdevMgrRemoveAllLabels(udevMgr, path);
 }
 
 return virSecurityDACSetOwnershipInternal(priv, src, path, uid, gid);
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index ecb4a40..2f07d6a 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -39,6 +39,7 @@ struct _virSecurityManager {
 virSecurityDriverPtr drv;
 unsigned int flags;
 const char *virtDriver;
+virUdevMgrPtr udevMgr;
 void *privateData;
 };
 
@@ -1001,3 +1002,18 @@ 
virSecurityManagerDomainSetPathLabel(virSecurityManagerPtr mgr,
 
 return 0;
 }
+
+
+void
+virSecurityManagerSetUdevManager(virSecurityManagerPtr mgr,
+ virUdevMgrPtr udevMgr)
+{
+mgr->udevMgr = virObjectRef(udevMgr);
+}
+
+
+virUdevMgrPtr
+virSecurityManagerGetUdevManager(virSecurityManagerPtr mgr)
+{
+return mgr->udevMgr;
+}
diff --git a/src/security/security_manager.h b/src/security/security_manager.h
index 4cbc2d8..8f565f7 100644
--- a/src/security/security_manager.h
+++ b/src/security/security_manager.h
@@ -26,6 +26,7 @@
 # include "domain_conf.h"
 # include "vircommand.h"
 # include "virstoragefile.h"
+# include "virudev.h"
 
 typedef struct _virSecurityManager virSecurityManager;
 typedef virSecurityManager *virSecurityManagerPtr;
@@ -164,4 +165,8 @@ int 
virSecurityManagerDomainSetPathLabel(virSecurityManagerPtr mg

[libvirt] [PATCH 09/17] virudev: Parse virUdevMgr from JSON

[Michal Privoznik]

Now that we are able to dump internal state into a JSON
file/string, we should be able to reverse the process and
reconstruct the internal state from a JSON file/string.

Signed-off-by: Michal Privoznik 
---
 src/libvirt_private.syms |   2 +
 src/util/virudev.c   | 159 +++
 src/util/virudev.h   |   2 +
 tests/virudevtest.c  |  45 ++
 4 files changed, 208 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b465a0d..3369600 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2578,6 +2578,8 @@ virUdevMgrAddLabel;
 virUdevMgrDumpFile;
 virUdevMgrDumpStr;
 virUdevMgrNew;
+virUdevMgrNewFromFile;
+virUdevMgrNewFromStr;
 virUdevMgrRemoveAllLabels;
 
 
diff --git a/src/util/virudev.c b/src/util/virudev.c
index 7f52149..d60fbf9 100644
--- a/src/util/virudev.c
+++ b/src/util/virudev.c
@@ -28,6 +28,7 @@
 #include "virhash.h"
 #include "virjson.h"
 #include "virobject.h"
+#include "virstring.h"
 #include "virudev.h"
 
 #define VIR_FROM_THIS VIR_FROM_SECURITY
@@ -176,6 +177,49 @@ udevSeclabelsDump(void *payload,
 }
 
 
+static udevSeclabelPtr
+udevSeclabelRestore(virJSONValuePtr labels)
+{
+udevSeclabelPtr ret = NULL;
+size_t i;
+
+if (VIR_ALLOC(ret) < 0)
+goto error;
+
+for (i = 0; i < virJSONValueArraySize(labels); i++) {
+virJSONValuePtr labelJSON = virJSONValueArrayGet(labels, i);
+virSecurityDeviceLabelDefPtr seclabel;
+const char *model;
+const char *label;
+
+if (!(model = virJSONValueObjectGetString(labelJSON, "model"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("seclabel missing model in JSON"));
+goto error;
+}
+
+if (!(label = virJSONValueObjectGetString(labelJSON, "label"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("seclabel missing label in JSON"));
+goto error;
+}
+
+if (!(seclabel = virSecurityDeviceLabelDefNewLabel(model, label)) ||
+udevSeclabelAppend(ret, seclabel) < 0) {
+virSecurityDeviceLabelDefFree(seclabel);
+goto error;
+}
+virSecurityDeviceLabelDefFree(seclabel);
+}
+
+return ret;
+
+ error:
+udevSeclabelFree(ret, NULL);
+return NULL;
+}
+
+
 static void
 virUdevMgrDispose(void *obj)
 {
@@ -359,3 +403,118 @@ virUdevMgrDumpFile(virUdevMgrPtr mgr,
 VIR_FREE(state);
 return ret;
 }
+
+
+static int
+virUdevRestoreLabels(virUdevMgrPtr mgr ATTRIBUTE_UNUSED,
+ virJSONValuePtr labelsArray)
+{
+int ret = -1;
+size_t i;
+udevSeclabelPtr list = NULL;
+
+for (i = 0; i < virJSONValueArraySize(labelsArray); i++) {
+virJSONValuePtr deviceJSON = virJSONValueArrayGet(labelsArray, i);
+virJSONValuePtr labels;
+const char *device;
+
+if (!(device = virJSONValueObjectGetString(deviceJSON, "device"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Missing device name"));
+goto cleanup;
+}
+
+if (!(labels = virJSONValueObjectGetArray(deviceJSON, "labels"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Missing device labels array"));
+goto cleanup;
+}
+
+if (!(list = udevSeclabelRestore(labels))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+   _("Malformed seclabels for device %s"), device);
+goto cleanup;
+}
+
+if (virHashAddEntry(mgr->labels, device, list) < 0)
+goto cleanup;
+list = NULL;
+}
+
+ret = 0;
+ cleanup:
+udevSeclabelFree(list, NULL);
+return ret;
+}
+
+
+static int
+virUdevMgrNewFromStrInternal(virUdevMgrPtr mgr,
+ const char *state)
+{
+virJSONValuePtr object;
+virJSONValuePtr child;
+int ret = -1;
+
+if (!(object = virJSONValueFromString(state)))
+goto cleanup;
+
+if (!(child = virJSONValueObjectGetArray(object, "labels"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Missing 'labels' object in JSON document"));
+goto cleanup;
+}
+
+if (virUdevRestoreLabels(mgr, child) < 0)
+goto cleanup;
+
+ret = 0;
+ cleanup:
+virJSONValueFree(object);
+return ret;
+}
+
+
+virUdevMgrPtr
+virUdevMgrNewFromStr(const char *str)
+{
+virUdevMgrPtr mgr;
+
+if (!(mgr = virUdevMgrNew()))
+goto error;
+
+if (virUdevMgrNewFromStrInternal(mgr, str) < 0)
+goto error;
+
+return mgr;
+ error:
+virObjectUnref(mgr);
+return NULL;
+}
+
+
+virUdevMgrPtr
+virUdevMgrNewFromFile(const char *filename)
+{
+virUdevMgrPtr mgr;
+char *state = NULL;
+
+if (!(mgr = virUdevMgrNew()))
+goto error;
+
+if (virFileReadAll(filename,
+

[libvirt] [PATCH 17/17] qemu: Filter uninteresting paths for virUdevMgr

[Michal Privoznik]

In case of udev, it will never try to reset security label on say
domain monitor socket, or some other channel. Therefore, it makes
sense to filter those paths out and keep the state file on the
disk small. The only paths that udev will handle are those
prefixed with "/dev/".

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_driver.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 51122d0..0fe91b9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -370,6 +370,15 @@ qemuSecurityChownCallback(virStorageSourcePtr src,
 
 
 static int
+qemuUdevFilter(const char *devpath,
+   const virSecurityDeviceLabelDef *seclabel ATTRIBUTE_UNUSED,
+   void *opaque ATTRIBUTE_UNUSED)
+{
+return STRPREFIX(devpath, "/dev/") ? 1 : 0;
+}
+
+
+static int
 qemuSecurityInit(virQEMUDriverPtr driver)
 {
 char **names;
@@ -390,6 +399,8 @@ qemuSecurityInit(virQEMUDriverPtr driver)
 if (!(driver->udevMgr = virUdevMgrNew()))
 goto error;
 }
+
+virUdevMgrSetFilter(driver->udevMgr, qemuUdevFilter, NULL);
 }
 
 if (cfg->allowDiskFormatProbing)
-- 
2.8.4

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

[libvirt] [PATCH 11/17] util: Introduce libvirt_udevhelper

[Michal Privoznik]

This is a small helper intended to be run by udev. On its input
(either as the only command line argument or in DEVNODE
environment vairable) it is given a device and on the output it
will either put nothing (meaning the device is not used by any of
the libvirt domains), or it will print out security labels in the
following form:

  UID GID SELABEL

Signed-off-by: Michal Privoznik 
---
 libvirt.spec.in   |   1 +
 mingw-libvirt.spec.in |   2 +
 po/POTFILES.in|   1 +
 src/Makefile.am   |  20 
 src/util/udevhelper.c | 137 ++
 5 files changed, 161 insertions(+)
 create mode 100644 src/util/udevhelper.c

diff --git a/libvirt.spec.in b/libvirt.spec.in
index 545990c..856c702 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1630,6 +1630,7 @@ exit 0
 %dir %attr(0700, root, root) %{_localstatedir}/log/libvirt/
 
 %attr(0755, root, root) %{_libexecdir}/libvirt_iohelper
+%attr(0755, root, root) %{_libexecdir}/libvirt_udevhelper
 
 %attr(0755, root, root) %{_sbindir}/libvirtd
 %attr(0755, root, root) %{_sbindir}/virtlogd
diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in
index c9bf503..015b06b 100644
--- a/mingw-libvirt.spec.in
+++ b/mingw-libvirt.spec.in
@@ -178,6 +178,8 @@ rm -rf $RPM_BUILD_ROOT%{mingw64_datadir}/gtk-doc/*
 
 rm -rf $RPM_BUILD_ROOT%{mingw32_libexecdir}/libvirt_iohelper.exe
 rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt_iohelper.exe
+rm -rf $RPM_BUILD_ROOT%{mingw32_libexecdir}/libvirt_udevhelper.exe
+rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt_udevhelper.exe
 rm -rf $RPM_BUILD_ROOT%{mingw32_libexecdir}/libvirt-guests.sh
 rm -rf $RPM_BUILD_ROOT%{mingw64_libexecdir}/libvirt-guests.sh
 
diff --git a/po/POTFILES.in b/po/POTFILES.in
index dabc612..7a89cbd 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -178,6 +178,7 @@ src/test/test_driver.c
 src/uml/uml_conf.c
 src/uml/uml_driver.c
 src/util/iohelper.c
+src/util/udevhelper.c
 src/util/viralloc.c
 src/util/viraudit.c
 src/util/virauth.c
diff --git a/src/Makefile.am b/src/Makefile.am
index 2ea6f2b..0c97728 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1007,6 +1007,9 @@ STORAGE_HELPER_DISK_SOURCES = 
\
 UTIL_IO_HELPER_SOURCES =   \
util/iohelper.c
 
+UTIL_UDEV_HELPER_SOURCES = \
+   util/udevhelper.c
+
 NETWORK_LEASES_HELPER_SOURCES = \
network/leaseshelper.c
 
@@ -2858,6 +2861,23 @@ libvirt_iohelper_CFLAGS = \
$(PIE_CFLAGS) \
$(NULL)
 
+libexec_PROGRAMS += libvirt_udevhelper
+libvirt_udevhelper_SOURCES = $(UTIL_UDEV_HELPER_SOURCES)
+libvirt_udevhelper_CFLAGS = \
+   $(AM_CFLAGS) \
+   $(PIE_CFLAGS) \
+   $(NULL)
+libvirt_udevhelper_LDFLAGS = \
+   $(AM_LDFLAGS) \
+   $(PIE_LDFLAGS) \
+   $(NULL)
+libvirt_udevhelper_LDADD = \
+   libvirt_util.la \
+   ../gnulib/lib/libgnu.la
+if WITH_DTRACE_PROBES
+libvirt_udevhelper_LDADD += libvirt_probes.lo
+endif WITH_DTRACE_PROBES
+
 if WITH_NETWORK
 libexec_PROGRAMS += libvirt_leaseshelper
 libvirt_leaseshelper_SOURCES = $(NETWORK_LEASES_HELPER_SOURCES)
diff --git a/src/util/udevhelper.c b/src/util/udevhelper.c
new file mode 100644
index 000..e44e6f6
--- /dev/null
+++ b/src/util/udevhelper.c
@@ -0,0 +1,137 @@
+/*
+ * Copyright (C) 2016 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ *
+ * Author: Michal Privoznik 
+ */
+
+#include 
+
+#include 
+#include 
+
+#include "configmake.h"
+#include "viralloc.h"
+#include "virgettext.h"
+#include "virobject.h"
+#include "virstring.h"
+#include "virthread.h"
+#include "virudev.h"
+#include "virutil.h"
+
+#define VIR_FROM_THIS VIR_FROM_SECURITY
+
+
+static void
+usage(const char *progname)
+{
+fprintf(stderr,
+_("Usage: %s [device]\n"
+  "\n"
+  "This program is intended to be run from udev to\n"
+  "maintain proper security labels on devices used by\n"
+  "domains managed by libvirt.\n"
+  "\n"
+  "For given device (either passed as the only\n"
+  "command line argument or set by DEVNODE

[libvirt] [PATCH 16/17] virudevtest: Introduce device filtering

[Michal Privoznik]

In some cases callers might want to filter what devices are
stored in this module (esp. when used in combination with udev
who cares about nothing but "/dev/" prefixed paths).

Signed-off-by: Michal Privoznik 
---
 src/libvirt_private.syms |  1 +
 src/util/virudev.c   | 25 ++
 src/util/virudev.h   | 13 
 tests/virudevtest.c  | 54 +++-
 4 files changed, 92 insertions(+), 1 deletion(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 66df1f7..60dc16b 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2584,6 +2584,7 @@ virUdevMgrNew;
 virUdevMgrNewFromFile;
 virUdevMgrNewFromStr;
 virUdevMgrRemoveAllLabels;
+virUdevMgrSetFilter;
 
 
 # util/viruri.h
diff --git a/src/util/virudev.c b/src/util/virudev.c
index 9940f5f..c17682c 100644
--- a/src/util/virudev.c
+++ b/src/util/virudev.c
@@ -36,6 +36,9 @@
 struct _virUdevMgr {
 virObjectLockable parent;
 virHashTablePtr labels;
+
+virUdevMgrFilter filter;
+void *opaque;
 };
 
 struct _udevSeclabel {
@@ -274,6 +277,17 @@ virUdevMgrAddLabel(virUdevMgrPtr mgr,
 
 virObjectLock(mgr);
 
+if (mgr->filter) {
+int rc = mgr->filter(device, seclabel, mgr->opaque);
+if (rc < 0)
+goto cleanup;
+if (rc == 0) {
+/* Claim success. */
+ret = 0;
+goto cleanup;
+}
+}
+
 if ((list = virHashLookup(mgr->labels, device))) {
 virSecurityDeviceLabelDefPtr entry = udevSeclabelFindByModel(list, 
seclabel->model);
 
@@ -561,3 +575,14 @@ virUdevMgrNewFromFile(const char *filename)
 VIR_FREE(state);
 return NULL;
 }
+
+void
+virUdevMgrSetFilter(virUdevMgrPtr mgr,
+virUdevMgrFilter filter,
+void *opaque)
+{
+virObjectLock(mgr);
+mgr->filter = filter;
+mgr->opaque = opaque;
+virObjectUnlock(mgr);
+}
diff --git a/src/util/virudev.h b/src/util/virudev.h
index 4e286bb..2f035e3 100644
--- a/src/util/virudev.h
+++ b/src/util/virudev.h
@@ -28,6 +28,15 @@
 typedef struct _virUdevMgr virUdevMgr;
 typedef virUdevMgr *virUdevMgrPtr;
 
+/* Filter some devices out in virUdevMgrAddLabel.
+ * Return 0 to NOT record label for device,
+ *1 to record the label for device,
+ *   -1 on error.
+ */
+typedef int (*virUdevMgrFilter)(const char *device,
+const virSecurityDeviceLabelDef *seclabel,
+void *opaque);
+
 virUdevMgrPtr virUdevMgrNew(void);
 virUdevMgrPtr virUdevMgrNewFromStr(const char *str);
 virUdevMgrPtr virUdevMgrNewFromFile(const char *filename);
@@ -47,4 +56,8 @@ char *virUdevMgrDumpStr(virUdevMgrPtr mgr);
 int virUdevMgrDumpFile(virUdevMgrPtr mgr,
const char *filename);
 
+void virUdevMgrSetFilter(virUdevMgrPtr mgr,
+ virUdevMgrFilter filter,
+ void *opaque);
+
 #endif
diff --git a/tests/virudevtest.c b/tests/virudevtest.c
index 36a9077..cd5d136 100644
--- a/tests/virudevtest.c
+++ b/tests/virudevtest.c
@@ -29,6 +29,7 @@
 struct testUdevData {
 const char *file;
 const char *const *labels;
+virUdevMgrFilter filter;
 };
 
 
@@ -49,6 +50,8 @@ testDump(const void *opaque)
 if (!(mgr = virUdevMgrNew()))
 goto cleanup;
 
+virUdevMgrSetFilter(mgr, data->filter, NULL);
+
 tmp = data->labels;
 while (*tmp) {
 const char *device;
@@ -194,6 +197,25 @@ testLookup(const void *opaque)
 
 
 static int
+filterAll(const char *device ATTRIBUTE_UNUSED,
+  const virSecurityDeviceLabelDef *seclabel ATTRIBUTE_UNUSED,
+  void *opaque ATTRIBUTE_UNUSED)
+{
+return 0;
+}
+
+
+static int
+filterAllowSda(const char *device,
+   const virSecurityDeviceLabelDef *seclabel ATTRIBUTE_UNUSED,
+   void *opaque ATTRIBUTE_UNUSED)
+{
+return STRPREFIX(device, "/dev/sda") ? 1 : 0;
+}
+
+
+
+static int
 mymain(void)
 {
 int ret = 0;
@@ -202,7 +224,7 @@ mymain(void)
 do {\
 const char *labels[] = {__VA_ARGS__, NULL}; \
 struct testUdevData data = {\
-.file = filename, .labels = labels, \
+.file = filename, .labels = labels, .filter = NULL, \
 };  \
 if (virTestRun("Dump " filename, testDump, &data) < 0)  \
 ret = -1;   \
@@ -227,6 +249,17 @@ mymain(void)
 ret = -1;   \
 } while (0)
 
+#define DO_TEST_FILTER(filename, fltr, ...) \
+do {\
+const char *labels[] = {__VA_ARGS__, NULL}; \
+struct testUdevData data = {  

[libvirt] [PATCH 00/17] Avoid races with udev

[Michal Privoznik]

I've came across interesting bug recently. The problem was that
user tried to start a domain, but qemu was denied access to some
device. Even though we relabelled it initially. By debugging I
found the root cause: while we were starting qemu, udev came and
restored original security labels. Sigh. We have two options
here:

a) write out series of udev rules so that whenever it tries to
relabel something our rule will stop it from doing so

b) write a small helper binary that will udev call in order to:
  1) detect whether device is in use by libvirt
  2) get seclabel that was set by libvirt

These patches implement the latter approach. While these patches
make life easier for us, there is still a race when udev might
restore the device's seclabel before we had chance to flush
internal database of seclabels for the helper binary. This is
something I'm currently focusing on. But before I get there, here
are patches that makes the problem much more bearable.

In case you want to try these patches, here are some scratch builds:

  https://mprivozn.fedorapeople.org/udev/

Also, you can find them on my branch:

  https://github.com/zippy2/libvirt/commits/udev_labels2


This beast is turned off by default, to turn it on you'll need to add:

  write_udev=1

to qemu.conf.


Michal Privoznik (17):
  virseclabel.h: Include stdbool.h
  virseclabel: Introduce virSecurityDeviceLabelDefNewLabel
  security_dac: Pass manager to virSecurityDACSetImageLabel
  security_dac: Pass manager to virSecurityDACRestoreFileLabelInternal
  virudev: Introduce basic skeleton
  virudev: Implement virUdevMgrAddLabel and virUdevMgrRemoveAllLabels
  virudev: Introduce virUdevMgrDump
  tests: Introduce virudevtest
  virudev: Parse virUdevMgr from JSON
  virudev: Introduce virUdevMgrLookupLabels
  util: Introduce libvirt_udevhelper
  security: Wire up virUdevMgr
  qemu.conf: Introduce write_udev
  qemu: Wire up virUdevMgr
  qemu: Reload virUdevMgr on start
  virudevtest: Introduce device filtering
  qemu: Filter uninteresting paths for virUdevMgr

 libvirt.spec.in   |   1 +
 mingw-libvirt.spec.in |   2 +
 po/POTFILES.in|   2 +
 src/Makefile.am   |  21 ++
 src/libvirt_private.syms  |  15 +
 src/qemu/libvirtd_qemu.aug|   1 +
 src/qemu/qemu.conf|   5 +
 src/qemu/qemu_conf.c  |   3 +
 src/qemu/qemu_conf.h  |   5 +
 src/qemu/qemu_domain.c|  12 +-
 src/qemu/qemu_domain.h|   3 +-
 src/qemu/qemu_driver.c|  40 +-
 src/qemu/qemu_hotplug.c   |  35 +-
 src/qemu/qemu_process.c   |  47 ++-
 src/qemu/qemu_process.h   |   3 +
 src/qemu/test_libvirtd_qemu.aug.in|   1 +
 src/security/security_dac.c   | 103 --
 src/security/security_manager.c   |  16 +
 src/security/security_manager.h   |   5 +
 src/security/security_selinux.c   |  47 ++-
 src/util/udevhelper.c | 137 +++
 src/util/virseclabel.c|  14 +
 src/util/virseclabel.h|   6 +
 src/util/virudev.c| 588 ++
 src/util/virudev.h|  63 
 tests/Makefile.am |  12 +
 tests/virudevmock.c   |  29 ++
 tests/virudevtest.c   | 312 
 tests/virudevtestdata/complex.json|  30 ++
 tests/virudevtestdata/empty.json  |   5 +
 tests/virudevtestdata/simple-dac.json |  13 +
 tests/virudevtestdata/simple-selinux.json |  13 +
 32 files changed, 1535 insertions(+), 54 deletions(-)
 create mode 100644 src/util/udevhelper.c
 create mode 100644 src/util/virudev.c
 create mode 100644 src/util/virudev.h
 create mode 100644 tests/virudevmock.c
 create mode 100644 tests/virudevtest.c
 create mode 100644 tests/virudevtestdata/complex.json
 create mode 100644 tests/virudevtestdata/empty.json
 create mode 100644 tests/virudevtestdata/simple-dac.json
 create mode 100644 tests/virudevtestdata/simple-selinux.json

-- 
2.8.4

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

[libvirt] [PATCH 15/17] qemu: Reload virUdevMgr on start

[Michal Privoznik]

If the daemon is restarted, the virUdevMgr loses its internal
state. This is because entries to its internal DB are added
whilst setting security labels. This obviously doesn't happen
when the daemon is restarted.
It's not wise to start with a fresh internal state and possibly
leave behind previous one.

Signed-off-by: Michal Privoznik 
---
 src/qemu/qemu_driver.c | 18 +++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index cc34782..51122d0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -377,10 +377,20 @@ qemuSecurityInit(virQEMUDriverPtr driver)
 virSecurityManagerPtr stack = NULL;
 virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
 unsigned int flags = 0;
+char *file = NULL;
 
-if (cfg->writeUdev &&
-!(driver->udevMgr = virUdevMgrNew()))
-goto error;
+if (cfg->writeUdev) {
+if (!(file = qemuProcessGetUdevPath(driver)))
+goto error;
+
+if (virFileExists(file)) {
+if (!(driver->udevMgr = virUdevMgrNewFromFile(file)))
+goto error;
+} else {
+if (!(driver->udevMgr = virUdevMgrNew()))
+goto error;
+}
+}
 
 if (cfg->allowDiskFormatProbing)
 flags |= VIR_SECURITY_MANAGER_ALLOW_DISK_PROBE;
@@ -442,12 +452,14 @@ qemuSecurityInit(virQEMUDriverPtr driver)
 }
 
 driver->securityManager = stack;
+VIR_FREE(file);
 virObjectUnref(cfg);
 return 0;
 
  error:
 virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Failed to initialize security drivers"));
+VIR_FREE(file);
 virObjectUnref(stack);
 virObjectUnref(mgr);
 virObjectUnref(cfg);
-- 
2.8.4

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

[libvirt] [PATCH 04/17] security_dac: Pass manager to virSecurityDACRestoreFileLabelInternal

[Michal Privoznik]

This change alone is not needed, but it prepares environment for
subsequent patches where we will need virSecurityManager much
deeper in the code.

Signed-off-by: Michal Privoznik 
---
 src/security/security_dac.c | 34 +++---
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 97be862..7f17124 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -377,10 +377,11 @@ virSecurityDACSetOwnership(virSecurityManagerPtr mgr,
 
 
 static int
-virSecurityDACRestoreFileLabelInternal(virSecurityDACDataPtr priv,
+virSecurityDACRestoreFileLabelInternal(virSecurityManagerPtr mgr,
virStorageSourcePtr src,
const char *path)
 {
+virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
 int rv;
 uid_t uid = 0;  /* By default return to root:root */
 gid_t gid = 0;
@@ -405,10 +406,10 @@ 
virSecurityDACRestoreFileLabelInternal(virSecurityDACDataPtr priv,
 
 
 static int
-virSecurityDACRestoreFileLabel(virSecurityDACDataPtr priv,
+virSecurityDACRestoreFileLabel(virSecurityManagerPtr mgr,
const char *path)
 {
-return virSecurityDACRestoreFileLabelInternal(priv, NULL, path);
+return virSecurityDACRestoreFileLabelInternal(mgr, NULL, path);
 }
 
 
@@ -515,7 +516,7 @@ virSecurityDACRestoreImageLabelInt(virSecurityManagerPtr 
mgr,
 }
 }
 
-return virSecurityDACRestoreFileLabelInternal(priv, src, NULL);
+return virSecurityDACRestoreFileLabelInternal(mgr, src, NULL);
 }
 
 
@@ -693,8 +694,7 @@ virSecurityDACRestorePCILabel(virPCIDevicePtr dev 
ATTRIBUTE_UNUSED,
   void *opaque)
 {
 virSecurityManagerPtr mgr = opaque;
-virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
-return virSecurityDACRestoreFileLabel(priv, file);
+return virSecurityDACRestoreFileLabel(mgr, file);
 }
 
 
@@ -704,8 +704,7 @@ virSecurityDACRestoreUSBLabel(virUSBDevicePtr dev 
ATTRIBUTE_UNUSED,
   void *opaque)
 {
 virSecurityManagerPtr mgr = opaque;
-virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
-return virSecurityDACRestoreFileLabel(priv, file);
+return virSecurityDACRestoreFileLabel(mgr, file);
 }
 
 
@@ -715,8 +714,7 @@ virSecurityDACRestoreSCSILabel(virSCSIDevicePtr dev 
ATTRIBUTE_UNUSED,
void *opaque)
 {
 virSecurityManagerPtr mgr = opaque;
-virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
-return virSecurityDACRestoreFileLabel(priv, file);
+return virSecurityDACRestoreFileLabel(mgr, file);
 }
 
 
@@ -908,7 +906,6 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr,
   virDomainChrDefPtr dev,
   virDomainChrSourceDefPtr dev_source)
 {
-virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
 virSecurityDeviceLabelDefPtr chr_seclabel = NULL;
 char *in = NULL, *out = NULL;
 int ret = -1;
@@ -923,7 +920,7 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr mgr,
 switch ((virDomainChrType) dev_source->type) {
 case VIR_DOMAIN_CHR_TYPE_DEV:
 case VIR_DOMAIN_CHR_TYPE_FILE:
-ret = virSecurityDACRestoreFileLabel(priv, dev_source->data.file.path);
+ret = virSecurityDACRestoreFileLabel(mgr, dev_source->data.file.path);
 break;
 
 case VIR_DOMAIN_CHR_TYPE_PIPE:
@@ -931,10 +928,10 @@ virSecurityDACRestoreChardevLabel(virSecurityManagerPtr 
mgr,
 virAsprintf(&in, "%s.in", dev_source->data.file.path) < 0)
 goto done;
 if (virFileExists(in) && virFileExists(out)) {
-if (virSecurityDACRestoreFileLabel(priv, out) < 0 ||
-virSecurityDACRestoreFileLabel(priv, in) < 0)
+if (virSecurityDACRestoreFileLabel(mgr, out) < 0 ||
+virSecurityDACRestoreFileLabel(mgr, in) < 0)
 goto done;
-} else if (virSecurityDACRestoreFileLabel(priv, 
dev_source->data.file.path) < 0) {
+} else if (virSecurityDACRestoreFileLabel(mgr, 
dev_source->data.file.path) < 0) {
 goto done;
 }
 ret = 0;
@@ -1053,12 +1050,11 @@ virSecurityDACRestoreInputLabel(virSecurityManagerPtr 
mgr,
 virDomainDefPtr def ATTRIBUTE_UNUSED,
 virDomainInputDefPtr input)
 {
-virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
 int ret = -1;
 
 switch ((virDomainInputType) input->type) {
 case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH:
-ret = virSecurityDACRestoreFileLabel(priv, input->source.evdev);
+ret = virSecurityDACRestoreFileLabel(mgr, input->source.evdev);
 break;
 
 case VIR_DOMAIN_INPUT_TYPE_MOUSE:
@@ -1126,7 +1122,7 @@ virSecurityDACResto

[libvirt] [PATCH 05/17] virudev: Introduce basic skeleton

[Michal Privoznik]

This is new internal class that is going to remember
 pairs. Moreover, it is going to be
able to flush the pairs into a file so that a helper (which is
introduced later in the series) can look into the file and answer
question: "Is this path in use by libvirt and if so what security
labels should it have?"

You can say that we already have security drivers for that. And
you would be right. But unfortunately on a Linux system, some
processes running in it reset security labels sometimes, possibly
cutting of a running domain. For instance udev. There has been a
problem (race you can say), where libvirt set seclabels on a disk
device, and wanted to start a domain but meanwhile udev came and
restored the seclabels.

With this module we can have a small helper that could be used by
udev to find out what seclabels should a device have.

Signed-off-by: Michal Privoznik 
---
 src/Makefile.am  |  1 +
 src/libvirt_private.syms |  4 +++
 src/util/virudev.c   | 68 
 src/util/virudev.h   | 31 ++
 4 files changed, 104 insertions(+)
 create mode 100644 src/util/virudev.c
 create mode 100644 src/util/virudev.h

diff --git a/src/Makefile.am b/src/Makefile.am
index 8ee5567..2ea6f2b 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -179,6 +179,7 @@ UTIL_SOURCES =  
\
util/virtime.h util/virtime.c   \
util/virtpm.h util/virtpm.c \
util/virtypedparam.c util/virtypedparam.h   \
+   util/virudev.c util/virudev.h   \
util/virusb.c util/virusb.h \
util/viruri.h util/viruri.c \
util/virutil.c util/virutil.h   \
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index bec2628..5ae8037 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2573,6 +2573,10 @@ virTypedParamsSerialize;
 virTypedParamsValidate;
 
 
+# util/virudev.h
+virUdevMgrNew;
+
+
 # util/viruri.h
 virURIFormat;
 virURIFormatParams;
diff --git a/src/util/virudev.c b/src/util/virudev.c
new file mode 100644
index 000..66b5a58
--- /dev/null
+++ b/src/util/virudev.c
@@ -0,0 +1,68 @@
+/*
+ * virudev.c: udev rules engine
+ *
+ * Copyright (C) 2016 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ *
+ * Author: Michal Privoznik 
+ */
+
+#include 
+
+#include "virudev.h"
+#include "virobject.h"
+
+struct _virUdevMgr {
+virObjectLockable parent;
+};
+
+static virClassPtr virUdevMgrClass;
+
+
+static void
+virUdevMgrDispose(void *obj ATTRIBUTE_UNUSED)
+{
+/* nada */
+}
+
+
+static int virUdevMgrOnceInit(void)
+{
+if (!(virUdevMgrClass = virClassNew(virClassForObjectLockable(),
+"virUdevMgr",
+sizeof(virUdevMgr),
+virUdevMgrDispose)))
+return -1;
+
+return 0;
+}
+
+
+VIR_ONCE_GLOBAL_INIT(virUdevMgr)
+
+
+virUdevMgrPtr virUdevMgrNew(void)
+{
+virUdevMgrPtr mgr;
+
+if (virUdevMgrInitialize() < 0)
+return NULL;
+
+if (!(mgr = virObjectLockableNew(virUdevMgrClass)))
+return NULL;
+
+return mgr;
+}
diff --git a/src/util/virudev.h b/src/util/virudev.h
new file mode 100644
index 000..28e336f
--- /dev/null
+++ b/src/util/virudev.h
@@ -0,0 +1,31 @@
+/*
+ * virudev.h: udev rules engine
+ *
+ * Copyright (C) 2016 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library.  If not, see
+ * .
+ *
+ * Author: Michal Privoznik 
+ */
+
+#ifndef __VIR_UDEV_H__
+# define __VI

[libvirt] [PATCH 06/17] virudev: Implement virUdevMgrAddLabel and virUdevMgrRemoveAllLabels

[Michal Privoznik]

These APIs start to implement what was laid out in the module
description. We need to be able to store given security label (on
domain startup, device attach), and then remove all security
labels associated with it (on domain shutdown, device detach).

Signed-off-by: Michal Privoznik 
---
 src/libvirt_private.syms |   2 +
 src/util/virudev.c   | 142 ++-
 src/util/virudev.h   |   8 +++
 3 files changed, 149 insertions(+), 3 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5ae8037..184317a 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2574,7 +2574,9 @@ virTypedParamsValidate;
 
 
 # util/virudev.h
+virUdevMgrAddLabel;
 virUdevMgrNew;
+virUdevMgrRemoveAllLabels;
 
 
 # util/viruri.h
diff --git a/src/util/virudev.c b/src/util/virudev.c
index 66b5a58..f4799e7 100644
--- a/src/util/virudev.c
+++ b/src/util/virudev.c
@@ -22,20 +22,101 @@
 
 #include 
 
-#include "virudev.h"
+#include "internal.h"
+#include "viralloc.h"
+#include "virhash.h"
 #include "virobject.h"
+#include "virudev.h"
+
+#define VIR_FROM_THIS VIR_FROM_SECURITY
 
 struct _virUdevMgr {
 virObjectLockable parent;
+virHashTablePtr labels;
 };
 
+struct _udevSeclabel {
+virSecurityDeviceLabelDefPtr *seclabels;
+size_t nseclabels;
+};
+
+typedef struct _udevSeclabel udevSeclabel;
+typedef udevSeclabel *udevSeclabelPtr;
+
 static virClassPtr virUdevMgrClass;
 
 
+static virSecurityDeviceLabelDefPtr
+udevSeclabelFindByModel(udevSeclabelPtr list,
+const char *model)
+{
+size_t i;
+
+for (i = 0; list && i < list->nseclabels; i++) {
+if (STREQ(list->seclabels[i]->model, model))
+return list->seclabels[i];
+}
+
+return NULL;
+}
+
+
+static int
+udevSeclabelAppend(udevSeclabelPtr list,
+   const virSecurityDeviceLabelDef *seclabel)
+{
+virSecurityDeviceLabelDefPtr copy = 
virSecurityDeviceLabelDefCopy(seclabel);
+if (VIR_APPEND_ELEMENT_COPY(list->seclabels, list->nseclabels, copy) < 0) {
+virSecurityDeviceLabelDefFree(copy);
+return -1;
+}
+return 0;
+}
+
+
+static void
+udevSeclabelFree(void *payload,
+ const void *name ATTRIBUTE_UNUSED)
+{
+udevSeclabelPtr list = payload;
+size_t i;
+
+if (!list)
+return;
+
+for (i = 0; i < list->nseclabels; i++)
+virSecurityDeviceLabelDefFree(list->seclabels[i]);
+VIR_FREE(list->seclabels);
+VIR_FREE(list);
+}
+
+
+static int
+udevSeclabelUpdate(udevSeclabelPtr list,
+   const virSecurityDeviceLabelDef *seclabel)
+{
+size_t i;
+
+for (i = 0; list && i < list->nseclabels; i++) {
+if (STREQ(list->seclabels[i]->model, seclabel->model)) {
+virSecurityDeviceLabelDefPtr copy = 
virSecurityDeviceLabelDefCopy(seclabel);
+if (!copy)
+return -1;
+virSecurityDeviceLabelDefFree(list->seclabels[i]);
+list->seclabels[i] = copy;
+return 0;
+}
+}
+
+return -1;
+}
+
+
 static void
-virUdevMgrDispose(void *obj ATTRIBUTE_UNUSED)
+virUdevMgrDispose(void *obj)
 {
-/* nada */
+virUdevMgrPtr mgr = obj;
+virHashFree(mgr->labels);
 }
 
 
@@ -64,5 +145,60 @@ virUdevMgrPtr virUdevMgrNew(void)
 if (!(mgr = virObjectLockableNew(virUdevMgrClass)))
 return NULL;
 
+if (!(mgr->labels = virHashCreate(10, udevSeclabelFree)))
+goto error;
+
 return mgr;
+
+ error:
+virObjectUnref(mgr);
+return NULL;
+}
+
+
+int
+virUdevMgrAddLabel(virUdevMgrPtr mgr,
+   const char *device,
+   const virSecurityDeviceLabelDef *seclabel)
+{
+int ret = -1;
+udevSeclabelPtr list;
+
+virObjectLock(mgr);
+
+if ((list = virHashLookup(mgr->labels, device))) {
+virSecurityDeviceLabelDefPtr entry = udevSeclabelFindByModel(list, 
seclabel->model);
+
+if (entry) {
+udevSeclabelUpdate(list, seclabel);
+} else {
+if (udevSeclabelAppend(list, seclabel) < 0)
+goto cleanup;
+}
+} else {
+if (VIR_ALLOC(list) < 0 ||
+udevSeclabelAppend(list, seclabel) < 0 ||
+virHashAddEntry(mgr->labels, device, list) < 0) {
+udevSeclabelFree(list, NULL);
+goto cleanup;
+}
+}
+
+ret = 0;
+ cleanup:
+virObjectUnlock(mgr);
+return ret;
+}
+
+
+int
+virUdevMgrRemoveAllLabels(virUdevMgrPtr mgr,
+  const char *device)
+{
+int ret = -1;
+
+virObjectLock(mgr);
+ret = virHashRemoveEntry(mgr->labels, device);
+virObjectUnlock(mgr);
+return ret;
 }
diff --git a/src/util/virudev.h b/src/util/virudev.h
index 28e336f..692b39b 100644
--- a/src/util/virudev.h
+++ b/src/util/virudev.h
@@ -23,9 +23,17 @@
 #ifndef __VIR_UDEV_H__
 # define __VIR_UDEV_H__
 
+# include "virseclabel.h"
+
 typedef struct _virUdevMgr virUdevMgr;
 

[libvirt] [PATCH 02/17] virseclabel: Introduce virSecurityDeviceLabelDefNewLabel

[Michal Privoznik]

So far, this function is not needed with the current code. But it
is going to do so in subsequent commits. We already have
virSecurityDeviceLabelDefNew() which sets just model for new
seclabel, this new API sets both model and label.

Signed-off-by: Michal Privoznik 
---
 src/libvirt_private.syms |  1 +
 src/util/virseclabel.c   | 14 ++
 src/util/virseclabel.h   |  4 
 3 files changed, 19 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index b4210f4..bec2628 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2303,6 +2303,7 @@ virSCSIDeviceSetUsedBy;
 # util/virseclabel.h
 virSecurityDeviceLabelDefFree;
 virSecurityDeviceLabelDefNew;
+virSecurityDeviceLabelDefNewLabel;
 virSecurityLabelDefFree;
 virSecurityLabelDefNew;
 
diff --git a/src/util/virseclabel.c b/src/util/virseclabel.c
index 02a8342..d762a15 100644
--- a/src/util/virseclabel.c
+++ b/src/util/virseclabel.c
@@ -83,6 +83,20 @@ virSecurityDeviceLabelDefNew(const char *model)
 return seclabel;
 }
 
+virSecurityDeviceLabelDefPtr
+virSecurityDeviceLabelDefNewLabel(const char *model,
+  const char *label)
+{
+virSecurityDeviceLabelDefPtr seclabel;
+
+if (!(seclabel = virSecurityDeviceLabelDefNew(model)) ||
+VIR_STRDUP(seclabel->label, label) < 0) {
+virSecurityDeviceLabelDefFree(seclabel);
+seclabel = NULL;
+}
+
+return seclabel;
+}
 
 virSecurityDeviceLabelDefPtr
 virSecurityDeviceLabelDefCopy(const virSecurityDeviceLabelDef *src)
diff --git a/src/util/virseclabel.h b/src/util/virseclabel.h
index 558d4eb..a0af1fd 100644
--- a/src/util/virseclabel.h
+++ b/src/util/virseclabel.h
@@ -64,6 +64,10 @@ virSecurityDeviceLabelDefPtr
 virSecurityDeviceLabelDefNew(const char *model);
 
 virSecurityDeviceLabelDefPtr
+virSecurityDeviceLabelDefNewLabel(const char *model,
+  const char *label);
+
+virSecurityDeviceLabelDefPtr
 virSecurityDeviceLabelDefCopy(const virSecurityDeviceLabelDef *src)
 ATTRIBUTE_NONNULL(1);
 
-- 
2.8.4

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

[libvirt] libvirt compiles on RISC-V (RV64G)

[Richard W.M. Jones]

I'm happy to announce that libvirt compiles fine from git on
Fedora/RISC-V.  This has little or no practical value at all, since
RISC-V lacks such essentials such as virtualization, qemu etc.
However I suppose you could use it as a remote client.

# file src/.libs/libvirt.so.0.2004.0 
src/.libs/libvirt.so.0.2004.0: ELF 64-bit LSB shared object, UCB RISC-V, 
version 1 (SYSV), dynamically linked, 
BuildID[sha1]=525ed42ce6d1284c6a909bd6f1b0d6181e88af7b, not stripped

# file tools/.libs/virsh 
tools/.libs/virsh: ELF 64-bit LSB shared object, UCB RISC-V, version 1 (SYSV), 
dynamically linked, interpreter /lib/ld.so.1, for GNU/Linux 2.6.32, 
BuildID[sha1]=67e2b69f9007c02137545fed1b7fd9b2871740ee, not stripped

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

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

Re: [libvirt] [RFC PATCH] libxl: add tunnelled migration support

[Joao Martins]

On 10/26/2016 07:33 AM, Bob Liu wrote:
> Tunnelled migration doesn't require any extra network connections beside the
> libvirt daemon.
> It's capable of strong encryption and is the default option in openstack-nova.
> 
> This patch add the tunnelled migration(Tunnel3params) support to libxl.
> The data flow in the src side is:
>  * libxlDoMigrateSend() -> pipe
>  * libxlTunnel3MigrationFunc() poll pipe out and then write to dest stream.
> 
> While in the dest side:
> Stream -> pipe -> 'recvfd of libxlDomainStartRestore'
> 
> The usage is the same as p2p migration, execpt adding one more '--tunnelled' 
> to
  ^^ except
> the libvirt p2p migration command.
> 
> Signed-off-by: Bob Liu 
Nice :) Now openstack no longer needs to have tunnelled flag removed on nova to
get migration working.

See some comments below, its a first review as I would still like to test it.

> ---
>  src/libxl/libxl_driver.c|  58 ++-
>  src/libxl/libxl_migration.c | 241 
> +---
>  src/libxl/libxl_migration.h |   9 ++
>  3 files changed, 292 insertions(+), 16 deletions(-)
> 
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index b66cb1f..a01bbff 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -5918,6 +5918,61 @@ libxlDomainMigrateBegin3Params(virDomainPtr domain,
>  }
>  
>  static int
> +libxlDomainMigratePrepareTunnel3Params(virConnectPtr dconn,
> +   virStreamPtr st,
> +   virTypedParameterPtr params,
> +   int nparams,
> +   const char *cookiein,
> +   int cookieinlen,
> +   char **cookieout ATTRIBUTE_UNUSED,
> +   int *cookieoutlen ATTRIBUTE_UNUSED,
> +   unsigned int flags)
> +{
> +libxlDriverPrivatePtr driver = dconn->privateData;
> +virDomainDefPtr def = NULL;
> +const char *dom_xml = NULL;
> +const char *dname = NULL;
> +const char *uri_in = NULL;
> +
> +#ifdef LIBXL_HAVE_NO_SUSPEND_RESUME
> +virReportUnsupportedError();
> +return -1;
> +#endif
> +
> +virCheckFlags(LIBXL_MIGRATION_FLAGS, -1);
> +if (virTypedParamsValidate(params, nparams, LIBXL_MIGRATION_PARAMETERS) 
> < 0)
> +goto error;
> +
> +if (virTypedParamsGetString(params, nparams,
> +VIR_MIGRATE_PARAM_DEST_XML,
> +&dom_xml) < 0 ||
> +virTypedParamsGetString(params, nparams,
> +VIR_MIGRATE_PARAM_DEST_NAME,
> +&dname) < 0 ||
> +virTypedParamsGetString(params, nparams,
> +VIR_MIGRATE_PARAM_URI,
> +&uri_in) < 0)
> +
> +goto error;
> +
> +if (!(def = libxlDomainMigrationPrepareDef(driver, dom_xml, dname)))
> +goto error;
> +
> +if (virDomainMigratePrepareTunnel3ParamsEnsureACL(dconn, def) < 0)
> +goto error;
> +
> +if (libxlDomainMigrationPrepareTunnel3(dconn, st, &def, cookiein,
> +   cookieinlen, flags) < 0)
> +goto error;
> +
> +return 0;
> +
> + error:
> +virDomainDefFree(def);
> +return -1;
> +}
> +
> +static int
>  libxlDomainMigratePrepare3Params(virConnectPtr dconn,
>   virTypedParameterPtr params,
>   int nparams,
> @@ -6017,7 +6072,7 @@ libxlDomainMigratePerform3Params(virDomainPtr dom,
>  if (virDomainMigratePerform3ParamsEnsureACL(dom->conn, vm->def) < 0)
>  goto cleanup;
>  
> -if (flags & VIR_MIGRATE_PEER2PEER) {
> +if ((flags & (VIR_MIGRATE_TUNNELLED | VIR_MIGRATE_PEER2PEER))) {
>  if (libxlDomainMigrationPerformP2P(driver, vm, dom->conn, dom_xml,
> dconnuri, uri, dname, flags) < 0)
>  goto cleanup;
> @@ -6501,6 +6556,7 @@ static virHypervisorDriver libxlHypervisorDriver = {
>  .nodeDeviceReset = libxlNodeDeviceReset, /* 1.2.3 */
>  .domainMigrateBegin3Params = libxlDomainMigrateBegin3Params, /* 1.2.6 */
>  .domainMigratePrepare3Params = libxlDomainMigratePrepare3Params, /* 
> 1.2.6 */
> +.domainMigratePrepareTunnel3Params = 
> libxlDomainMigratePrepareTunnel3Params, /* 2.3.1 */
The version here is incorrect. It should be the next one to be tagged (after the
ongoing freeze). Which means 2.5.0. Note that the versioning used has changed a
bit: major number is incremented per year, minor per month and bugfix number for
-maint releases.

>  .domainMigratePerform3Params = libxlDomainMigratePerform3Params, /* 
> 1.2.6 */
>  .domainMigrateFinish3Params = libxlDomainMigrateFinish3Params, /* 1.2.6 
> */
>  .domainMigrateCon

[libvirt] [PATCH] qemu: Minimalize global driver accesses

[Michal Privoznik]

Whilst working on another issue, I've noticed that in some
functions we have a local @driver variable among with access to
global @qemu_driver variable. This makes no sense.

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

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index bacabd2..3495cbf 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3831,7 +3831,7 @@ qemuDomainScreenshot(virDomainPtr dom,
 }
 unlink_tmp = true;
 
-virSecurityManagerSetSavedStateLabel(qemu_driver->securityManager, 
vm->def, tmp);
+virSecurityManagerSetSavedStateLabel(driver->securityManager, vm->def, 
tmp);
 
 qemuDomainObjEnterMonitor(driver, vm);
 if (qemuMonitorScreendump(priv->mon, tmp) < 0) {
@@ -11449,7 +11449,7 @@ qemuDomainMemoryPeek(virDomainPtr dom,
 goto endjob;
 }
 
-virSecurityManagerSetSavedStateLabel(qemu_driver->securityManager, 
vm->def, tmp);
+virSecurityManagerSetSavedStateLabel(driver->securityManager, vm->def, 
tmp);
 
 priv = vm->privateData;
 qemuDomainObjEnterMonitor(driver, vm);
-- 
2.8.4

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

[libvirt] [PATCH v4 2/6] conf, qemu: Add newer shmem models

[Martin Kletzander]

The old ivshmem is deprecated in QEMU, so let's use the better
ivshmem-{plain,doorbell} variants instead.

Signed-off-by: Martin Kletzander 
---
 docs/formatdomain.html.in | 10 +++---
 docs/schemas/domaincommon.rng |  2 ++
 src/conf/domain_conf.c|  4 +++-
 src/conf/domain_conf.h|  2 ++
 src/qemu/qemu_command.c   |  7 +++
 5 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 10a692de2553..11b3330cee87 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -6826,10 +6826,11 @@ qemu-kvm -net nic,model=? /dev/null
   ...
   
 
+  
   4
 
 
-  
+  
   2
   
   
@@ -6848,8 +6849,11 @@ qemu-kvm -net nic,model=? /dev/null
 
   Attribute type of the optional element model
   specifies the model of the underlying device providing the
-  shmem device.  Currently the only supported model is
-  ivshmem.
+  shmem device.  The models currently supported are
+  ivshmem (supports both server and server-less shmem, but is
+  deprecated by newer QEMU in favour of the -plain and -doorbell variants),
+  ivshmem-plain (only for server-less shmem) and
+  ivshmem-doorbell (only for shmem with the server).
 
 size
 
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 99e0eb6cb448..19d45fd6eae3 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3596,6 +3596,8 @@
 
   
 ivshmem
+ivshmem-plain
+ivshmem-doorbell
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 108a48ee974e..a233c0c4208a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -845,7 +845,9 @@ VIR_ENUM_IMPL(virDomainMemoryModel, 
VIR_DOMAIN_MEMORY_MODEL_LAST,
   "", "dimm")

 VIR_ENUM_IMPL(virDomainShmemModel, VIR_DOMAIN_SHMEM_MODEL_LAST,
-  "ivshmem")
+  "ivshmem",
+  "ivshmem-plain",
+  "ivshmem-doorbell")

 static virClassPtr virDomainObjClass;
 static virClassPtr virDomainXMLOptionClass;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index d2a9289e077d..541b6003c87e 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1564,6 +1564,8 @@ struct _virDomainNVRAMDef {

 typedef enum {
 VIR_DOMAIN_SHMEM_MODEL_IVSHMEM,
+VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_PLAIN,
+VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_DOORBELL,

 VIR_DOMAIN_SHMEM_MODEL_LAST
 } virDomainShmemModel;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index f05f2fd1c5c4..558122b4cf7e 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8504,6 +8504,13 @@ qemuBuildShmemCommandLine(virLogManagerPtr logManager,
 devstr = qemuBuildShmemDevLegacyStr(def, shmem, qemuCaps);
 break;

+case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_PLAIN:
+case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_DOORBELL:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("%s device is not supported with this QEMU binary"),
+   virDomainShmemModelTypeToString(shmem->model));
+break;
+
 case VIR_DOMAIN_SHMEM_MODEL_LAST:
 break;
 }
-- 
2.10.1

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

[libvirt] [PATCH v4 4/6] qemu: Save various defaults for shmem

[Martin Kletzander]

We're keeping some things at default and that's not something we want to
do intentionally.  Let's save some sensible defaults upfront in order to
avoid having problems later.  The details for the defaults (of the newer
implementation) can be found in qemu's commit 5400c02b90bb:

  http://git.qemu.org/?p=qemu.git;a=commit;h=5400c02b90bb

Since we are merely saving the defaults it will not change the guest ABI
and thanks to the fact that we're doing it in the PostParse callback it
will not break the ABI stability checks.

Signed-off-by: Martin Kletzander 
---
 src/qemu/qemu_domain.c | 54 ++
 tests/qemuxml2argvdata/qemuxml2argv-shmem.args |  2 +-
 .../qemuxml2xmlout-shmem-plain-doorbell.xml|  5 +-
 tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem.xml  |  1 +
 4 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 94f793e8ce9c..ee3bf8cae3a2 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2671,6 +2671,56 @@ qemuDomainChrDefDropDefaultPath(virDomainChrDefPtr chr,


 static int
+qemuDomainShmemDefPostParse(virDomainShmemDefPtr shm)
+{
+/* This was the default since the introduction of this device. */
+if (shm->model != VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_DOORBELL && !shm->size)
+shm->size = 4 << 20;
+
+/* Nothing more to check/change for IVSHMEM */
+if (shm->model == VIR_DOMAIN_SHMEM_MODEL_IVSHMEM)
+return 0;
+
+if (!shm->server.enabled) {
+if (shm->model == VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_DOORBELL) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("shmem model '%s' is supported "
+ "only with server option enabled"),
+   virDomainShmemModelTypeToString(shm->model));
+return -1;
+}
+
+if (shm->msi.enabled) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("shmem model '%s' doesn't support "
+ "msi"),
+   virDomainShmemModelTypeToString(shm->model));
+}
+} else {
+if (shm->model == VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_PLAIN) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("shmem model '%s' is supported "
+ "only with server option disabled"),
+   virDomainShmemModelTypeToString(shm->model));
+return -1;
+}
+
+if (shm->size) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("shmem model '%s' does not support size setting"),
+   virDomainShmemModelTypeToString(shm->model));
+return -1;
+}
+shm->msi.enabled = true;
+if (!shm->msi.ioeventfd)
+shm->msi.ioeventfd = VIR_TRISTATE_SWITCH_ON;
+}
+
+return 0;
+}
+
+
+static int
 qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
  const virDomainDef *def,
  virCapsPtr caps,
@@ -2874,6 +2924,10 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
 }
 }

+if (dev->type == VIR_DOMAIN_DEVICE_SHMEM &&
+qemuDomainShmemDefPostParse(dev->data.shmem) < 0)
+goto cleanup;
+
 ret = 0;

  cleanup:
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-shmem.args 
b/tests/qemuxml2argvdata/qemuxml2argv-shmem.args
index 99fac119b04c..bdf660a3c435 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-shmem.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-shmem.args
@@ -17,7 +17,7 @@ QEMU_AUDIO_DRV=none \
 -no-acpi \
 -boot c \
 -usb \
--device ivshmem,id=shmem0,shm=shmem0,bus=pci.0,addr=0x3 \
+-device ivshmem,id=shmem0,size=4m,shm=shmem0,bus=pci.0,addr=0x3 \
 -device ivshmem,id=shmem1,size=128m,shm=shmem1,bus=pci.0,addr=0x5 \
 -device ivshmem,id=shmem2,size=256m,shm=shmem2,bus=pci.0,addr=0x4 \
 -device ivshmem,id=shmem3,size=512m,chardev=charshmem3,bus=pci.0,addr=0x6 \
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem-plain-doorbell.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem-plain-doorbell.xml
index ab9c69bfccd4..7872e1c8a102 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem-plain-doorbell.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem-plain-doorbell.xml
@@ -23,6 +23,7 @@
 
 
   
+  4
   
 
 
@@ -38,11 +39,13 @@
 
   
   
+  
   
 
 
   
   
+  
   
 
 
@@ -54,7 +57,7 @@
 
   
   
-  
+  
   
 
 
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem.xml 
b/tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem.xml
index 5602913648bc..04b463a27892 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem.xml
@@ -23,6 +23,7 @@
 
 
   
+  4
   
 
 
-- 
2

[libvirt] [PATCH v4 6/6] qemu: Add support for hot/cold-(un)plug of shmem devices

[Martin Kletzander]

This is needed in order to migrate a domain with shmem devices as that
is not allowed to migrate.

Signed-off-by: Martin Kletzander 
---
 src/qemu/qemu_driver.c |  39 +++-
 src/qemu/qemu_hotplug.c| 248 -
 src/qemu/qemu_hotplug.h|   6 +
 tests/qemuhotplugtest.c|  21 ++
 .../qemuhotplug-ivshmem-doorbell-detach.xml|   7 +
 .../qemuhotplug-ivshmem-doorbell.xml   |   4 +
 .../qemuhotplug-ivshmem-plain-detach.xml   |   6 +
 .../qemuhotplug-ivshmem-plain.xml  |   3 +
 ...muhotplug-base-live+ivshmem-doorbell-detach.xml |   1 +
 .../qemuhotplug-base-live+ivshmem-doorbell.xml |  65 ++
 .../qemuhotplug-base-live+ivshmem-plain-detach.xml |   1 +
 .../qemuhotplug-base-live+ivshmem-plain.xml|  58 +
 12 files changed, 454 insertions(+), 5 deletions(-)
 create mode 100644 
tests/qemuhotplugtestdevices/qemuhotplug-ivshmem-doorbell-detach.xml
 create mode 100644 
tests/qemuhotplugtestdevices/qemuhotplug-ivshmem-doorbell.xml
 create mode 100644 
tests/qemuhotplugtestdevices/qemuhotplug-ivshmem-plain-detach.xml
 create mode 100644 tests/qemuhotplugtestdevices/qemuhotplug-ivshmem-plain.xml
 create mode 12 
tests/qemuhotplugtestdomains/qemuhotplug-base-live+ivshmem-doorbell-detach.xml
 create mode 100644 
tests/qemuhotplugtestdomains/qemuhotplug-base-live+ivshmem-doorbell.xml
 create mode 12 
tests/qemuhotplugtestdomains/qemuhotplug-base-live+ivshmem-plain-detach.xml
 create mode 100644 
tests/qemuhotplugtestdomains/qemuhotplug-base-live+ivshmem-plain.xml

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index bacabd2d348f..c164c390eccf 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7601,6 +7601,15 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
 dev->data.memory = NULL;
 break;

+case VIR_DOMAIN_DEVICE_SHMEM:
+ret = qemuDomainAttachShmemDevice(driver, vm,
+  dev->data.shmem);
+if (!ret) {
+alias = dev->data.shmem->info.alias;
+dev->data.shmem = NULL;
+}
+break;
+
 case VIR_DOMAIN_DEVICE_NONE:
 case VIR_DOMAIN_DEVICE_FS:
 case VIR_DOMAIN_DEVICE_INPUT:
@@ -7612,7 +7621,6 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
 case VIR_DOMAIN_DEVICE_SMARTCARD:
 case VIR_DOMAIN_DEVICE_MEMBALLOON:
 case VIR_DOMAIN_DEVICE_NVRAM:
-case VIR_DOMAIN_DEVICE_SHMEM:
 case VIR_DOMAIN_DEVICE_TPM:
 case VIR_DOMAIN_DEVICE_PANIC:
 case VIR_DOMAIN_DEVICE_IOMMU:
@@ -7690,6 +7698,9 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
 case VIR_DOMAIN_DEVICE_MEMORY:
 ret = qemuDomainDetachMemoryDevice(driver, vm, dev->data.memory);
 break;
+case VIR_DOMAIN_DEVICE_SHMEM:
+ret = qemuDomainDetachShmemDevice(driver, vm, dev->data.shmem);
+break;

 case VIR_DOMAIN_DEVICE_FS:
 case VIR_DOMAIN_DEVICE_INPUT:
@@ -7701,7 +7712,6 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
 case VIR_DOMAIN_DEVICE_SMARTCARD:
 case VIR_DOMAIN_DEVICE_MEMBALLOON:
 case VIR_DOMAIN_DEVICE_NVRAM:
-case VIR_DOMAIN_DEVICE_SHMEM:
 case VIR_DOMAIN_DEVICE_REDIRDEV:
 case VIR_DOMAIN_DEVICE_NONE:
 case VIR_DOMAIN_DEVICE_TPM:
@@ -7848,6 +7858,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
 virDomainControllerDefPtr controller;
 virDomainFSDefPtr fs;
 virDomainRedirdevDefPtr redirdev;
+virDomainShmemDefPtr shmem;

 switch ((virDomainDeviceType) dev->type) {
 case VIR_DOMAIN_DEVICE_DISK:
@@ -7972,6 +7983,18 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
 dev->data.redirdev = NULL;
 break;

+case VIR_DOMAIN_DEVICE_SHMEM:
+shmem = dev->data.shmem;
+if (virDomainShmemDefFind(vmdef, shmem) >= 0) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+   _("device is already in the domain configuration"));
+return -1;
+}
+if (virDomainShmemDefInsert(vmdef, shmem) < 0)
+return -1;
+dev->data.shmem = NULL;
+break;
+
 case VIR_DOMAIN_DEVICE_INPUT:
 case VIR_DOMAIN_DEVICE_SOUND:
 case VIR_DOMAIN_DEVICE_VIDEO:
@@ -7981,7 +8004,6 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
 case VIR_DOMAIN_DEVICE_SMARTCARD:
 case VIR_DOMAIN_DEVICE_MEMBALLOON:
 case VIR_DOMAIN_DEVICE_NVRAM:
-case VIR_DOMAIN_DEVICE_SHMEM:
 case VIR_DOMAIN_DEVICE_NONE:
 case VIR_DOMAIN_DEVICE_TPM:
 case VIR_DOMAIN_DEVICE_PANIC:
@@ -8128,6 +8150,16 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
 virDomainRedirdevDefFree(virDomainRedirdevDefRemove(vmdef, idx));
 break;

+case VIR_DOMAIN_DEVICE_SHMEM:
+if ((idx = virDomainShmemDefFind(vmdef, dev->data.shmem)) < 0) {
+virReportError(VIR_ERR_OPERATION_FAILED, "%s",
+   

[libvirt] [PATCH v4 3/6] qemu: Add capabilities for ivshmem-{plain, doorbell}

[Martin Kletzander]

Signed-off-by: Martin Kletzander 
---
 src/qemu/qemu_capabilities.c   |  5 ++
 src/qemu/qemu_capabilities.h   |  4 ++
 .../caps_2.6.0-gicv2.aarch64.xml   |  2 +
 .../caps_2.6.0-gicv3.aarch64.xml   |  2 +
 tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml  |  2 +
 tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml   |  2 +
 tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml   |  2 +
 .../qemuxml2argv-shmem-plain-doorbell.args | 43 ++
 .../qemuxml2argv-shmem-plain-doorbell.xml  | 58 +++
 .../qemuxml2xmlout-shmem-plain-doorbell.xml| 67 ++
 tests/qemuxml2xmltest.c|  1 +
 11 files changed, 188 insertions(+)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.xml
 create mode 100644 
tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem-plain-doorbell.xml

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 7a8202ad507d..cd2abf758357 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -347,6 +347,9 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "machine-iommu",
   "virtio-vga",
   "drive-iotune-max-length",
+  "ivshmem-plain",
+
+  "ivshmem-doorbell", /* 240 */
 );


@@ -1588,6 +1591,8 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
 { "pxb-pcie", QEMU_CAPS_DEVICE_PXB_PCIE },
 { "tls-creds-x509", QEMU_CAPS_OBJECT_TLS_CREDS_X509 },
 { "intel-iommu", QEMU_CAPS_DEVICE_INTEL_IOMMU },
+{ "ivshmem-plain", QEMU_CAPS_DEVICE_IVSHMEM_PLAIN },
+{ "ivshmem-doorbell", QEMU_CAPS_DEVICE_IVSHMEM_DOORBELL },
 };

 static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 6e7a8550a2a5..983ea758744e 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -381,6 +381,10 @@ typedef enum {
 QEMU_CAPS_MACHINE_IOMMU, /* -machine iommu=on */
 QEMU_CAPS_DEVICE_VIRTIO_VGA, /* -device virtio-vga */
 QEMU_CAPS_DRIVE_IOTUNE_MAX_LENGTH, /* -drive bps_max_length = and friends 
*/
+QEMU_CAPS_DEVICE_IVSHMEM_PLAIN, /* -device ivshmem-plain */
+
+/* 240 */
+QEMU_CAPS_DEVICE_IVSHMEM_DOORBELL, /* -device ivshmem-doorbell */

 QEMU_CAPS_LAST /* this must always be the last item */
 } virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml 
b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
index 5c6a70998c43..4f1a58b450a8 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv2.aarch64.xml
@@ -159,6 +159,8 @@
   
   
   
+  
+  
   2005094
   0
   
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml 
b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
index 6ba97be87458..0fa333a5fd1d 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0-gicv3.aarch64.xml
@@ -159,6 +159,8 @@
   
   
   
+  
+  
   2005094
   0
   
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml 
b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml
index 9174f58cdfd4..7b3b61c70d3e 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml
@@ -154,6 +154,8 @@
   
   
   
+  
+  
   2005094
   0
   
diff --git a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml
index 1c309dfadc4c..74b5402899b7 100644
--- a/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml
@@ -193,6 +193,8 @@
   
   
   
+  
+  
   2006000
   0
   
diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml 
b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml
index 2f168da53456..05dfcadb6ae2 100644
--- a/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml
@@ -194,6 +194,8 @@
   
   
   
+  
+  
   2007000
   0
(v2.7.0)
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.args 
b/tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.args
new file mode 100644
index ..7abc7f8c4be5
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.args
@@ -0,0 +1,43 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/home/test \
+USER=test \
+LOGNAME=test \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-name QEMUGuest1 \
+-S \
+-M pc \
+-m 214 \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/lib/domain--1-QEMUGuest1/monitor.sock,server,nowait \
+-no-acpi \
+-boot c \
+-usb \
+-object memory-backend-file,id=shmmem-shmem0,mem-path=/dev/shm/shmem0,\
+

[libvirt] [PATCH v4 5/6] qemu: Support newer ivshmem device variants

[Martin Kletzander]

QEMU added support for ivshmem-plain and ivshmem-doorbell.  Those are
reworked varians of legacy ivshmem that are compatible from the guest
POV, but not from host's POV and have sane specification and handling.

Details about the newer device type can be found in qemu's commit
5400c02b90bb:

  http://git.qemu.org/?p=qemu.git;a=commit;h=5400c02b90bb

Signed-off-by: Martin Kletzander 
---
 src/qemu/qemu_command.c  | 99 ++--
 src/qemu/qemu_command.h  | 10 +
 tests/qemuxml2argvtest.c |  3 ++
 3 files changed, 109 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 558122b4cf7e..972cccee25de 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8443,6 +8443,50 @@ qemuBuildShmemDevLegacyStr(virDomainDefPtr def,
 return NULL;
 }

+char *
+qemuBuildShmemDevStr(virDomainDefPtr def,
+ virDomainShmemDefPtr shmem,
+ virQEMUCapsPtr qemuCaps)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+if ((shmem->model == VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_PLAIN &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IVSHMEM_PLAIN)) ||
+(shmem->model == VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_DOORBELL &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IVSHMEM_DOORBELL))) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("shmem model '%s' is not supported "
+ "by this QEMU binary"),
+   virDomainShmemModelTypeToString(shmem->model));
+return NULL;
+}
+
+virBufferAsprintf(&buf, "%s", 
virDomainShmemModelTypeToString(shmem->model));
+virBufferAsprintf(&buf, ",id=%s", shmem->info.alias);
+
+if (shmem->server.enabled)
+virBufferAsprintf(&buf, ",chardev=char%s", shmem->info.alias);
+else
+virBufferAsprintf(&buf, ",memdev=shmmem-%s", shmem->info.alias);
+
+if (shmem->msi.vectors)
+virBufferAsprintf(&buf, ",vectors=%u", shmem->msi.vectors);
+if (shmem->msi.ioeventfd) {
+virBufferAsprintf(&buf, ",ioeventfd=%s",
+  virTristateSwitchTypeToString(shmem->msi.ioeventfd));
+}
+
+if (qemuBuildDeviceAddressStr(&buf, def, &shmem->info, qemuCaps) < 0) {
+virBufferFreeAndReset(&buf);
+return NULL;
+}
+
+if (virBufferCheckError(&buf) < 0)
+return NULL;
+
+return virBufferContentAndReset(&buf);
+}
+
 static char *
 qemuBuildShmemBackendChrStr(virLogManagerPtr logManager,
 virCommandPtr cmd,
@@ -8463,6 +8507,50 @@ qemuBuildShmemBackendChrStr(virLogManagerPtr logManager,
 return devstr;
 }

+
+virJSONValuePtr
+qemuBuildShmemBackendMemProps(virDomainShmemDefPtr shmem)
+{
+char *mem_path = NULL;
+virJSONValuePtr ret = NULL;
+
+if (virAsprintf(&mem_path, "/dev/shm/%s", shmem->name) < 0)
+return NULL;
+
+virJSONValueObjectCreate(&ret,
+ "s:mem-path", mem_path,
+ "U:size", shmem->size,
+ NULL);
+
+VIR_FREE(mem_path);
+return ret;
+}
+
+
+static char *
+qemuBuildShmemBackendMemStr(virDomainShmemDefPtr shmem)
+{
+char *ret = NULL;
+char *alias = NULL;
+virJSONValuePtr props = qemuBuildShmemBackendMemProps(shmem);
+
+if (!props)
+return NULL;
+
+if (virAsprintf(&alias, "shmmem-%s", shmem->info.alias) < 0)
+goto cleanup;
+
+ret = virQEMUBuildObjectCommandlineFromJSON("memory-backend-file",
+alias,
+props);
+ cleanup:
+VIR_FREE(alias);
+virJSONValueFree(props);
+
+return ret;
+}
+
+
 static int
 qemuBuildShmemCommandLine(virLogManagerPtr logManager,
   virCommandPtr cmd,
@@ -8505,10 +8593,15 @@ qemuBuildShmemCommandLine(virLogManagerPtr logManager,
 break;

 case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_PLAIN:
+if (!(devstr = qemuBuildShmemBackendMemStr(shmem)))
+return -1;
+
+virCommandAddArgList(cmd, "-object", devstr, NULL);
+VIR_FREE(devstr);
+
+/* fall-through */
 case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_DOORBELL:
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-   _("%s device is not supported with this QEMU binary"),
-   virDomainShmemModelTypeToString(shmem->model));
+devstr = qemuBuildShmemDevStr(def, shmem, qemuCaps);
 break;

 case VIR_DOMAIN_SHMEM_MODEL_LAST:
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 2f2a6ff877e7..facc833bf886 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -186,4 +186,14 @@ bool qemuCheckCCWS390AddressSupport(const virDomainDef 
*def,
 virJSONValuePtr qemuBuildHotpluggableCPUProps(const virDomainVcpuDef *vcpu)
 ATTRIBUTE_NONNULL(1);

+virJSONValuePtr qemuBuildShmemBackendMemProps(

[libvirt] [PATCH v4 0/6] Newer ivshmem models

[Martin Kletzander]

v4:
 - Incorporated John's review

v3:
 - https://www.redhat.com/archives/libvir-list/2016-September/msg01232.html


Martin Kletzander (6):
  conf, qemu: Add support for shmem model
  conf, qemu: Add newer shmem models
  qemu: Add capabilities for ivshmem-{plain,doorbell}
  qemu: Save various defaults for shmem
  qemu: Support newer ivshmem device variants
  qemu: Add support for hot/cold-(un)plug of shmem devices

 docs/formatdomain.html.in  |  12 +
 docs/schemas/domaincommon.rng  |  11 +
 src/conf/domain_conf.c |  46 +++-
 src/conf/domain_conf.h |  10 +
 src/libvirt_private.syms   |   2 +
 src/qemu/qemu_capabilities.c   |   5 +
 src/qemu/qemu_capabilities.h   |   4 +
 src/qemu/qemu_command.c| 111 -
 src/qemu/qemu_command.h|  10 +
 src/qemu/qemu_domain.c |  54 +
 src/qemu/qemu_driver.c |  39 +++-
 src/qemu/qemu_hotplug.c| 248 -
 src/qemu/qemu_hotplug.h|   6 +
 .../caps_2.6.0-gicv2.aarch64.xml   |   2 +
 .../caps_2.6.0-gicv3.aarch64.xml   |   2 +
 tests/qemucapabilitiesdata/caps_2.6.0.ppc64le.xml  |   2 +
 tests/qemucapabilitiesdata/caps_2.6.0.x86_64.xml   |   2 +
 tests/qemucapabilitiesdata/caps_2.7.0.x86_64.xml   |   2 +
 tests/qemuhotplugtest.c|  21 ++
 .../qemuhotplug-ivshmem-doorbell-detach.xml|   7 +
 .../qemuhotplug-ivshmem-doorbell.xml   |   4 +
 .../qemuhotplug-ivshmem-plain-detach.xml   |   6 +
 .../qemuhotplug-ivshmem-plain.xml  |   3 +
 ...muhotplug-base-live+ivshmem-doorbell-detach.xml |   1 +
 .../qemuhotplug-base-live+ivshmem-doorbell.xml |  65 ++
 .../qemuhotplug-base-live+ivshmem-plain-detach.xml |   1 +
 .../qemuhotplug-base-live+ivshmem-plain.xml|  58 +
 .../qemuxml2argv-shmem-plain-doorbell.args |  43 
 ...m.xml => qemuxml2argv-shmem-plain-doorbell.xml} |  16 +-
 tests/qemuxml2argvdata/qemuxml2argv-shmem.args |   2 +-
 tests/qemuxml2argvdata/qemuxml2argv-shmem.xml  |   2 +
 tests/qemuxml2argvtest.c   |   3 +
 ...xml => qemuxml2xmlout-shmem-plain-doorbell.xml} |  18 +-
 tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem.xml  |   9 +
 tests/qemuxml2xmltest.c|   1 +
 35 files changed, 798 insertions(+), 30 deletions(-)
 create mode 100644 
tests/qemuhotplugtestdevices/qemuhotplug-ivshmem-doorbell-detach.xml
 create mode 100644 
tests/qemuhotplugtestdevices/qemuhotplug-ivshmem-doorbell.xml
 create mode 100644 
tests/qemuhotplugtestdevices/qemuhotplug-ivshmem-plain-detach.xml
 create mode 100644 tests/qemuhotplugtestdevices/qemuhotplug-ivshmem-plain.xml
 create mode 12 
tests/qemuhotplugtestdomains/qemuhotplug-base-live+ivshmem-doorbell-detach.xml
 create mode 100644 
tests/qemuhotplugtestdomains/qemuhotplug-base-live+ivshmem-doorbell.xml
 create mode 12 
tests/qemuhotplugtestdomains/qemuhotplug-base-live+ivshmem-plain-detach.xml
 create mode 100644 
tests/qemuhotplugtestdomains/qemuhotplug-base-live+ivshmem-plain.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-shmem-plain-doorbell.args
 copy tests/qemuxml2argvdata/{qemuxml2argv-shmem.xml => 
qemuxml2argv-shmem-plain-doorbell.xml} (80%)
 copy tests/qemuxml2xmloutdata/{qemuxml2xmlout-shmem.xml => 
qemuxml2xmlout-shmem-plain-doorbell.xml} (82%)

--
2.10.1

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

[libvirt] [PATCH v4 1/6] conf, qemu: Add support for shmem model

[Martin Kletzander]

Just the default one now, new ones will be added in following commits.

Signed-off-by: Martin Kletzander 
---
 docs/formatdomain.html.in |  8 +
 docs/schemas/domaincommon.rng |  9 +
 src/conf/domain_conf.c| 44 +--
 src/conf/domain_conf.h|  8 +
 src/libvirt_private.syms  |  2 ++
 src/qemu/qemu_command.c   | 11 +-
 tests/qemuxml2argvdata/qemuxml2argv-shmem.xml |  2 ++
 tests/qemuxml2xmloutdata/qemuxml2xmlout-shmem.xml |  8 +
 8 files changed, 80 insertions(+), 12 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index c70377ba4847..10a692de2553 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -6829,6 +6829,7 @@ qemu-kvm -net nic,model=? /dev/null
   4
 
 
+  
   2
   
   
@@ -6843,6 +6844,13 @@ qemu-kvm -net nic,model=? /dev/null
   The shmem element has one mandatory attribute,
   name to identify the shared memory.
 
+model
+
+  Attribute type of the optional element model
+  specifies the model of the underlying device providing the
+  shmem device.  Currently the only supported model is
+  ivshmem.
+
 size
 
   The optional size element specifies the size of the shared
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index dba9187aa716..99e0eb6cb448 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3592,6 +3592,15 @@
   
   
 
+  
+
+  
+ivshmem
+  
+
+  
+
+
   
 
   
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 03506cbec142..108a48ee974e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -844,6 +844,9 @@ VIR_ENUM_IMPL(virDomainBlockJob, 
VIR_DOMAIN_BLOCK_JOB_TYPE_LAST,
 VIR_ENUM_IMPL(virDomainMemoryModel, VIR_DOMAIN_MEMORY_MODEL_LAST,
   "", "dimm")

+VIR_ENUM_IMPL(virDomainShmemModel, VIR_DOMAIN_SHMEM_MODEL_LAST,
+  "ivshmem")
+
 static virClassPtr virDomainObjClass;
 static virClassPtr virDomainXMLOptionClass;
 static void virDomainObjDispose(void *obj);
@@ -12351,6 +12354,20 @@ virDomainShmemDefParseXML(xmlNodePtr node,

 ctxt->node = node;

+tmp = virXPathString("string(./model/@type)", ctxt);
+if (tmp) {
+/* If there's none, we will automatically have the first one
+ * (as default).  Unfortunately this has to be done for
+ * compatibility reasons. */
+if ((def->model = virDomainShmemModelTypeFromString(tmp)) < 0) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("Unknown shmem model type '%s'"), tmp);
+goto cleanup;
+}
+
+VIR_FREE(tmp);
+}
+
 if (!(def->name = virXMLPropString(node, "name"))) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
_("shmem element must contain 'name' attribute"));
@@ -14939,6 +14956,9 @@ virDomainShmemDefEquals(virDomainShmemDefPtr src,
 if (src->size != dst->size)
 return false;

+if (src->model != dst->model)
+return false;
+
 if (src->server.enabled != dst->server.enabled)
 return false;

@@ -18927,6 +18947,15 @@ 
virDomainShmemDefCheckABIStability(virDomainShmemDefPtr src,
 return false;
 }

+if (src->model != dst->model) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+   _("Target shared memory model '%s' does not match "
+ "source model '%s'"),
+   virDomainShmemModelTypeToString(dst->model),
+   virDomainShmemModelTypeToString(src->model));
+return false;
+}
+
 if (src->size != dst->size) {
 virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Target shared memory size '%llu' does not match "
@@ -21980,20 +22009,13 @@ virDomainShmemDefFormat(virBufferPtr buf,
 virDomainShmemDefPtr def,
 unsigned int flags)
 {
-virBufferEscapeString(buf, "name);
-
-if (!def->size &&
-!def->server.enabled &&
-!def->msi.enabled &&
-!virDomainDeviceInfoNeedsFormat(&def->info, flags)) {
-virBufferAddLit(buf, "/>\n");
-return 0;
-} else {
-virBufferAddLit(buf, ">\n");
-}
+virBufferEscapeString(buf, "\n", def->name);

 virBufferAdjustIndent(buf, 2);

+virBufferAsprintf(buf, "\n",
+  virDomainShmemModelTypeToString(def->model));
+
 if (def->size)
 virBufferAsprintf(buf, "%llu\n

Re: [libvirt] [PATCH v11 5/5] qemu: Add secret object hotplug for TCP chardev TLS

[Pavel Hrdina]

On Mon, Oct 24, 2016 at 06:46:21PM -0400, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1300776
> 
> Complete the implementation of support for TLS encryption on
> chardev TCP transports by adding the hotplug ability of a secret
> to generate the passwordid for the TLS object for chrdev, RNG,
> and redirdev.
> 
> Likewise, add the ability to hot unplug that secret object as well
> 
> Signed-off-by: John Ferlan 
> ---
>  src/qemu/qemu_driver.c  |   6 +-
>  src/qemu/qemu_hotplug.c | 148 
> ++--
>  src/qemu/qemu_hotplug.h |   9 ++-
>  tests/qemuhotplugtest.c |   2 +-
>  4 files changed, 141 insertions(+), 24 deletions(-)

ACK with the ordering fixed.

Pavel


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

Re: [libvirt] [PATCH v11 4/5] qemu: Add a secret object to/for a char source dev

[Pavel Hrdina]

On Mon, Oct 24, 2016 at 06:46:20PM -0400, John Ferlan wrote:
> Add the secret object so the 'passwordid=' can be added if the command line
> if there's a secret defined in/on the host for TCP chardev TLS objects.
> 
> Preparation for the secret involves adding the secinfo to the char source
> device prior to command line processing. There are multiple possibilities
> for TCP chardev source backend usage.
> 
> Add test for at least a serial chardev as an example.
> 
> Signed-off-by: John Ferlan 
> ---
>  src/qemu/qemu_command.c|  30 +++-
>  src/qemu/qemu_command.h|   1 +
>  src/qemu/qemu_domain.c | 173 
> -
>  src/qemu/qemu_domain.h |  17 +-
>  src/qemu/qemu_hotplug.c|   1 +
>  src/qemu/qemu_process.c|   9 +-
>  ...xml2argv-serial-tcp-tlsx509-secret-chardev.args |  38 +
>  ...uxml2argv-serial-tcp-tlsx509-secret-chardev.xml |  50 ++
>  tests/qemuxml2argvtest.c   |  17 ++
>  9 files changed, 327 insertions(+), 9 deletions(-)
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.args
>  create mode 100644 
> tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.xml
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 6bf6510..1c07959 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -695,6 +695,7 @@ qemuBuildRBDSecinfoURI(virBufferPtr buf,
>   * @tlspath: path to the TLS credentials
>   * @listen: boolen listen for client or server setting
>   * @verifypeer: boolean to enable peer verification (form of authorization)
> + * @secalias: if one exists, the alias of the security object for passwordid
>   * @qemuCaps: capabilities
>   * @propsret: json properties to return
>   *
> @@ -706,6 +707,7 @@ int
>  qemuBuildTLSx509BackendProps(const char *tlspath,
>   bool isListen,
>   bool verifypeer,
> + const char *secalias,
>   virQEMUCapsPtr qemuCaps,
>   virJSONValuePtr *propsret)
>  {
> @@ -731,6 +733,10 @@ qemuBuildTLSx509BackendProps(const char *tlspath,
>   NULL) < 0)
>  goto cleanup;
>  
> +if (secalias &&
> +virJSONValueObjectAdd(*propsret, "s:passwordid", secalias, NULL) < 0)
> +goto cleanup;
> +
>  ret = 0;
>  
>   cleanup:
> @@ -745,6 +751,7 @@ qemuBuildTLSx509BackendProps(const char *tlspath,
>   * @tlspath: path to the TLS credentials
>   * @listen: boolen listen for client or server setting
>   * @verifypeer: boolean to enable peer verification (form of authorization)
> + * @addpasswordid: boolean to handle adding passwordid to object
>   * @inalias: Alias for the parent to generate object alias
>   * @qemuCaps: capabilities
>   *
> @@ -757,6 +764,7 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
>  const char *tlspath,
>  bool isListen,
>  bool verifypeer,
> +bool addpasswordid,
>  const char *inalias,
>  virQEMUCapsPtr qemuCaps)
>  {
> @@ -764,11 +772,16 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
>  char *objalias = NULL;
>  virJSONValuePtr props = NULL;
>  char *tmp = NULL;
> +char *secalias = NULL;
>  
> -if (qemuBuildTLSx509BackendProps(tlspath, isListen, verifypeer,
> - qemuCaps, &props) < 0)
> +if (addpasswordid &&
> +!(secalias = qemuDomainGetSecretAESAlias(inalias, false)))
>  return -1;
>  
> +if (qemuBuildTLSx509BackendProps(tlspath, isListen, verifypeer, secalias,
> + qemuCaps, &props) < 0)
> +goto cleanup;
> +
>  if (!(objalias = qemuAliasTLSObjFromChardevAlias(inalias)))
>  goto cleanup;
>  
> @@ -784,6 +797,7 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd,
>  virJSONValueFree(props);
>  VIR_FREE(objalias);
>  VIR_FREE(tmp);
> +VIR_FREE(secalias);
>  return ret;
>  }
>  
> @@ -4936,11 +4950,23 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
>  virBufferAdd(&buf, nowait ? ",server,nowait" : ",server", -1);
>  
>  if (dev->data.tcp.haveTLS == VIR_TRISTATE_BOOL_YES) {
> +qemuDomainChrSourcePrivatePtr chrSourcePriv =
> +QEMU_DOMAIN_CHR_SOURCE_PRIVATE(dev);
>  char *objalias = NULL;
>  
> +/* Add the secret object first if necessary. The
> + * secinfo is added only to a TCP serial device during
> + * qemuDomainSecretChardevPrepare. Subsequently called
> + * functions can just check the config fields */
> +if (chrSourcePriv 

Re: [libvirt] [PATCH v5 03/15] qemu: set/use info->pciConnectFlags when validating/assigning PCI addresses

[Andrea Bolognani]

On Tue, 2016-10-25 at 14:08 -0400, Laine Stump wrote:
> > [...]
> > > @@ -926,7 +857,7 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def 
> > > ATTRIBUTE_UNUSED,
> > >entireSlot = (addr->function == 0 &&
> > >  addr->multi != VIR_TRISTATE_SWITCH_ON);
> > >
> > > -if (virDomainPCIAddressReserveAddr(addrs, addr, flags,
> > > +if (virDomainPCIAddressReserveAddr(addrs, addr, 
> > > info->pciConnectFlags,
> > >   entireSlot, true) < 0)
> > Would it be cleaner to have a qemuDomainPCIAddressReserveAddr()
> > function that takes @info directly?
> 
> Actually in a later series (the one that cleans up the *Slot() vs 
> *Addr() naming), I eliminated all but one of the 
> qemuDomainPCIAddressReserve*() functions anyway. After that series, 
> there are only two *PCIAddressReserve*() functions used in this file: 
> qemuDomainPCIAddressReserveNextAddr() (21 times), and 
> virDomainPCIAddressReserveAddr() (12 times). The latter can't have a 
> nice flags-removing wrapper added in qemu_domain_address.c (like the 
> former does) because it often is called with a bare address - no DeviceInfo
> 
> (Well, I don't know, maybe it could be done by reorganizing some of the 
> calls, I'll have to look at it).
> 
> > It would be used only a single time, so it could very well be
> > argued that it would be overkill... On the other hand, it would
> > be neat not to use virDomainPCIAddressReserve*() functions at
> > all in the qemu driver and rely solely on the wrappers instead.
> > 
> > Speaking of which, even with the full series applied there
> > are still a bunch of uses of virDomainPCIAddressReserveAddr()
> > and virDomainPCIAddressReserveSlot(), mostly in
> > qemuDomainValidateDevicePCISlots{PIIX3,Q35}().
> 
> Yeah, my later series turns all of those into 
> virDomainPCIAddressReserveAddr().

Sorry, I haven't looked at any of your follow-up series at
all yet. Disregard my comments then :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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