Re: [libvirt] [PATCH v5 6/9] conf: Add new secret element for tcp chardev

[John Ferlan]

On 08/05/2016 04:25 AM, Daniel P. Berrange wrote:
> On Thu, Aug 04, 2016 at 11:21:24AM -0400, John Ferlan wrote:
>> Define, parse, and format a key secret element for a chardev tcp backend.
>> This secret will be used in conjunction with the chartcp_tls_x509_cert_dir
>> in order to provide the secret to the TLS encrypted TCP chardev.
>>
>> 
>>
>> Signed-off-by: John Ferlan 
>> ---
>>  docs/formatdomain.html.in  | 29 
>>  docs/schemas/domaincommon.rng  | 21 +
>>  src/conf/domain_conf.c | 35 +++
>>  src/conf/domain_conf.h |  3 ++
>>  ...uxml2argv-serial-tcp-tlsx509-secret-chardev.xml | 51 
>> ++
>>  ...ml2xmlout-serial-tcp-tlsx509-secret-chardev.xml |  1 +
>>  tests/qemuxml2xmltest.c|  1 +
>>  7 files changed, 141 insertions(+)
>>  create mode 100644 
>> tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-tlsx509-secret-chardev.xml
>>  create mode 12 
>> tests/qemuxml2xmloutdata/qemuxml2xmlout-serial-tcp-tlsx509-secret-chardev.xml
> 
> Hmm, it feels little odd that we're having to give the password in
> the XML, for a certificate thats configured in qemu.conf. I wonder
> if we instead need to have the secret UUID listed in qemu.conf too
> 
> 

I knew there was something I wanted to get back to...

I guess it seemed awkward to have to modify qemu.conf to list a UUID of
a libvirt secret that would be generated after initial startup and thus
would require a restart to read/load the secret into the cfg.

I suppose that's akin to having/changing the "{spice|vnc}_password" in
qemu.conf, so perhaps no different from that processing. Still

Hmmm... I suppose the admin interface could handle these tasks as well.

Anyway - secondarily, by adding UUID to qemu.conf, if cfg->chardevTLS
was set (something I appear to have forgotten to do in patch 2 too,
sigh), then that would mean every domain would use TLS. Is that desired?
Or should there still be some domain XML attribute added to signify the
desire for the domain to use TLS.

Would there ever be a use case where multiple TLS environments would be
set up for different domains with the same host?

Tks -

John

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

[libvirt] [PATCH] util: Quiet the logging if perf file doesn't exist

[John Ferlan]

Commit id 'b00d7f29' shifted the opening of the /sys/devices/intel_cqm/type
file from event enable to perf event initialization. If the file did not
exist, then an error would be written to the domain log:

2016-09-06 20:51:21.677+: 7310: error : virFileReadAll:1360 : Failed to 
open file '/sys/devices/intel_cqm/type': No such file or directory

Since the error is now handled in virPerfEventEnable by checking if the
event_attr->attrType == 0 for CMT, MBML, and MBMT events - we can just
use the Quiet API in order to not log the error we're going to throw away.

Additionally, rather than using virReportSystemError, use virReportError
and VIR_ERR_ARGUMENT_UNSUPPORTED in order to signify that support isn't there
for that type of perf event - adjust the error message as well.

Signed-off-by: John Ferlan 
---
 src/util/virperf.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/util/virperf.c b/src/util/virperf.c
index cd66b05..a65a4bf 100644
--- a/src/util/virperf.c
+++ b/src/util/virperf.c
@@ -116,7 +116,7 @@ virPerfRdtAttrInit(void)
 char *tmp = NULL;
 unsigned int attr_type = 0;
 
-if (virFileReadAll("/sys/devices/intel_cqm/type", 10, ) < 0)
+if (virFileReadAllQuiet("/sys/devices/intel_cqm/type", 10, ) < 0)
 goto error;
 
 if ((tmp = strchr(buf, '\n')))
@@ -174,9 +174,9 @@ virPerfEventEnable(virPerfPtr perf,
 if (event_attr->attrType == 0 && (type == VIR_PERF_EVENT_CMT ||
type == VIR_PERF_EVENT_MBMT ||
type == VIR_PERF_EVENT_MBML)) {
-virReportSystemError(errno,
- _("Unable to open perf event for %s"),
- virPerfEventTypeToString(event->type));
+virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+   _("unable to enable perf event for %s"),
+   virPerfEventTypeToString(event->type));
 return -1;
 }
 
-- 
2.7.4

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

Re: [libvirt] [Qemu-devel] [PATCH v7 0/4] Add Mediated device support

[Alex Williamson]

On Wed, 7 Sep 2016 01:05:11 +0530
Kirti Wankhede  wrote:

> On 9/6/2016 11:10 PM, Alex Williamson wrote:
> > On Sat, 3 Sep 2016 22:04:56 +0530
> > Kirti Wankhede  wrote:
> >   
> >> On 9/3/2016 3:18 AM, Paolo Bonzini wrote:  
> >>>
> >>>
> >>> On 02/09/2016 20:33, Kirti Wankhede wrote:
>   We could even do:
> >>
> >> echo $UUID1:$GROUPA > create
> >>
> >> where $GROUPA is the group ID of a previously created mdev device into
> >> which $UUID1 is to be created and added to the same group.
>  
> >>>
> >>> From the point of view of libvirt, I think I prefer Alex's idea.
> >>>  could be an additional element in the nodedev-create XML:
> >>>
> >>> 
> >>>   my-vgpu
> >>>   pci__86_00_0
> >>>   
> >>> 
> >>> 0695d332-7831-493f-9e71-1c85c8911a08
> >>> group1
> >>>   
> >>> 
> >>>
> >>> (should group also be a UUID?)
> >>> 
> >>
> >> No, this should be a unique number in a system, similar to iommu_group.  
> > 
> > Sorry, just trying to catch up on this thread after a long weekend.
> > 
> > We're talking about iommu groups here, we're not creating any sort of
> > parallel grouping specific to mdev devices.  
> 
> I thought we were talking about group of mdev devices and not iommu
> group. IIRC, there were concerns about it (this would be similar to
> UUID+instance) and that would (ab)use iommu groups.

What constraints does a group, which is not an iommu group, place on the
usage of the mdev devices?  What happens if we put two mdev devices in
the same "mdev group" and then assign them to separate VMs/users?  I
believe that the answer is that this theoretical "mdev group" doesn't
actually impose any constraints on the devices within the group or how
they're used.

vfio knows about iommu groups and we consider an iommu group to be the
unit of ownership for userspace.  Therefore by placing multiple mdev
devices within the same iommu group we can be assured that there's only
one user for that group.  Furthermore, the specific case for this
association on NVIDIA is to couple the hardware peer-to-peer resources
for the individual mdev devices.  Therefore this particular grouping
does imply a lack of isolation between those mdev devices involved in
the group.

For mdev devices which are actually isolated from one another, where
they don't poke these p2p holes, placing them in the same iommu group
is definitely an abuse of the interface and is going to lead to
problems with a single iommu context.  But how does libvirt know that
one type of mdev device needs to be grouped while another type doesn't?

There's really not much that I like about using iommu groups in this
way, it's just that they seem to solve this particular problem of
enforcing how such a group can be used and imposing a second form of
grouping onto the vfio infrastructure seems much too complex.
 
> I'm thinking about your suggestion, but would also like to know your
> thought how sysfs interface would look like? Its still no clear to me.
> Or will it be better to have grouping at mdev layer?

In previous replies I had proposed that a group could be an additional
argument when we write the mdev UUID to the create entry in sysfs.
This is specifically why I listed only the UUID when creating the first
mdev device and UUID:group when creating the second.  The user would
need to go determine the group ID allocated for the first entry to
specify creating the second within that same group.

I have no love for this proposal, it's functional but not elegant and
again leaves libvirt lost in trying to determine which devices need to
be grouped together and which have no business being grouped together.

Let's think through this further and let me make a couple assumptions
to get started:

1) iommu groups are the way that we want to group NVIDIA vGPUs because:
  a) The peer-to-peer resources represent an isolation gap between
 mdev devices, iommu groups represent sets of isolated devices.
  b) The 1:1 mapping of an iommu group to a user matches the NVIDIA
 device model.
  c) iommu_group_for_each_dev() gives the vendor driver the
 functionality it needs to perform a first-open/last-close
 device walk for configuring these p2p resources.

2) iommu groups as used by mdev devices should contain the minimum
number of devices in order to provide the maximum iommu context
flexibility.

Do we agree on these?  The corollary is that NVIDIA is going to suffer
reduced iommu granularity exactly because of the requirement to setup
p2p resources between mdev devices within the same VM.  This has
implications when guest iommus are in play (viommu).

So by default we want an iommu group per mdev.  This works for all mdev
devices as far as we know, including NVIDIA with the constraint that we
only have a single NVIDIA device per VM.

What if we want multiple NVIDIA devices?  We either need to create the
additional devices with 

[libvirt] [PATCH 2/2] storage: Need to properly read the crypt offset value

[John Ferlan]

Commit id 'a48c7141' altered how to determine if a volume was encrypted
by adding a peek at an offset into the file at a specific buffer location.
Unfortunately, all that was compared was the first "char" of the buffer
against the expect "int" value.

Restore the virReadBufInt32BE to get the complete field in order to
compare against the expected value from the qcow2EncryptionInfo or
qcow1EncryptionInfo "modeValue" field.

This restores the capability to create a volume with encryption, then
refresh the pool, and still find the encryption for the volume.

Signed-off-by: John Ferlan 
---
 src/util/virstoragefile.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 41827f0..272db67 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -888,7 +888,7 @@ virStorageFileHasEncryptionFormat(const struct 
FileEncryptionInfo *info,
   size_t len)
 {
 if (!info->magic && info->modeOffset == -1)
-return 0; /* Shouldn't happen - expect at least one */
+return false; /* Shouldn't happen - expect at least one */
 
 if (info->magic) {
 if (!virStorageFileMatchesMagic(info->magicOffset,
@@ -906,10 +906,13 @@ virStorageFileHasEncryptionFormat(const struct 
FileEncryptionInfo *info,
 
 return true;
 } else if (info->modeOffset != -1) {
+int crypt_format;
+
 if (info->modeOffset >= len)
 return false;
 
-if (buf[info->modeOffset] != info->modeValue)
+crypt_format = virReadBufInt32BE(buf + info->modeOffset);
+if (crypt_format != info->modeValue)
 return false;
 
 return true;
-- 
2.7.4

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

[libvirt] [PATCH 1/2] storage: Need to refresh secret for luks volume after volume refresh

[John Ferlan]

A LUKS volume uses the volume secret type just like the QCOW2 secret, so
adjust the loading of the default secrets to handle any volume that the
virStorageFileGetMetadataFromBuf code has deemed to be an encrypted volume
to search for the volume's secret. This lookup is done by volume usage
where the usage is expected to be the path to volume.

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

diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index ac6abbb..6c8bae2 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1270,8 +1270,8 @@ virStorageBackendFileSystemVolDelete(virConnectPtr conn 
ATTRIBUTE_UNUSED,
  * @conn: Connection pointer to fetch secret
  * @vol: volume being refreshed
  *
- * If the volume had a QCOW secret generated, we need to regenerate the
- * secret
+ * If the volume had a secret generated, we need to regenerate the
+ * encryption secret information
  *
  * Returns 0 if no secret or secret setup was successful,
  * -1 on failures w/ error message set
@@ -1283,12 +1283,16 @@ 
virStorageBackendFileSystemLoadDefaultSecrets(virConnectPtr conn,
 virSecretPtr sec;
 virStorageEncryptionSecretPtr encsec = NULL;
 
-/* Only necessary for qcow format */
-if (!vol->target.encryption ||
-vol->target.encryption->format != VIR_STORAGE_ENCRYPTION_FORMAT_QCOW ||
-vol->target.encryption->nsecrets != 0)
+if (!vol->target.encryption || vol->target.encryption->nsecrets != 0)
 return 0;
 
+/* The encryption secret for qcow2 and luks volumes use the path
+ * to the volume, so look for a secret with the path. If not found,
+ * then we cannot generate the secret after a refresh (or restart).
+ * This may be the case if someone didn't follow instructions and created
+ * a usage string that although matched with the secret usage string,
+ * didn't contain the path to the volume. We won't error in that case,
+ * but we also cannot find the secret. */
 if (!(sec = virSecretLookupByUsage(conn, VIR_SECRET_USAGE_TYPE_VOLUME,
vol->target.path)))
 return 0;
-- 
2.7.4

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

[libvirt] [PATCH 0/2] A couple of volume encryption patches

[John Ferlan]

I'm assume there will be a couple of bz's on these...

Patch 1 fixes a problem where the vol-dumpxml would not list the 
element from a volume in a pool after the initial creation and a pool
refresh (or libvirtd restart). This issue was missed due to rewriting
the LUKS support to not use it's own "new" secret type (key), but rather
use the existing "volume" secret type.

Patch 2 fixes a problem where QCOW2 (or QCOW1) encrypted volume would
lose the  and  after a volume refresh. Details are
in the patch. The issue is rooted in proper detection of the volume type
during the virStorageFileGetMetadataFromBuf call.

John Ferlan (2):
  storage: Need to refresh secret for luks volume after volume refresh
  storage: Need to properly read the crypt offset value

 src/storage/storage_backend_fs.c | 16 ++--
 src/util/virstoragefile.c|  7 +--
 2 files changed, 15 insertions(+), 8 deletions(-)

-- 
2.7.4

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

Re: [libvirt] [Qemu-devel] [PATCH v7 0/4] Add Mediated device support

[Kirti Wankhede]

On 9/6/2016 11:10 PM, Alex Williamson wrote:
> On Sat, 3 Sep 2016 22:04:56 +0530
> Kirti Wankhede  wrote:
> 
>> On 9/3/2016 3:18 AM, Paolo Bonzini wrote:
>>>
>>>
>>> On 02/09/2016 20:33, Kirti Wankhede wrote:  
  We could even do:  
>>
>> echo $UUID1:$GROUPA > create
>>
>> where $GROUPA is the group ID of a previously created mdev device into
>> which $UUID1 is to be created and added to the same group.  
   
>>>
>>> From the point of view of libvirt, I think I prefer Alex's idea.
>>>  could be an additional element in the nodedev-create XML:
>>>
>>> 
>>>   my-vgpu
>>>   pci__86_00_0
>>>   
>>> 
>>> 0695d332-7831-493f-9e71-1c85c8911a08
>>> group1
>>>   
>>> 
>>>
>>> (should group also be a UUID?)
>>>   
>>
>> No, this should be a unique number in a system, similar to iommu_group.
> 
> Sorry, just trying to catch up on this thread after a long weekend.
> 
> We're talking about iommu groups here, we're not creating any sort of
> parallel grouping specific to mdev devices.

I thought we were talking about group of mdev devices and not iommu
group. IIRC, there were concerns about it (this would be similar to
UUID+instance) and that would (ab)use iommu groups.

I'm thinking about your suggestion, but would also like to know your
thought how sysfs interface would look like? Its still no clear to me.
Or will it be better to have grouping at mdev layer?

Kirti.

>  This is why my example
> created a device and then required the user to go find the group number
> given to that device in order to create another device within the same
> group.  iommu group numbering is not within the user's control and is
> not a uuid.  libvirt can refer to the group as anything it wants in the
> xml, but the host group number is allocated by the host, not under user
> control, is not persistent.  libvirt would just be giving it a name to
> know which devices are part of the same group.  Perhaps the runtime xml
> would fill in the group number once created.
> 
> There were also a lot of unanswered questions in my proposal, it's not
> clear that there's a standard algorithm for when mdev devices need to
> be grouped together.  Should we even allow groups to span multiple host
> devices?  Should they be allowed to span devices from different
> vendors?
>
> If we imagine a scenario of a group composed of a mix of Intel and
> NVIDIA vGPUs, what happens when an Intel device is opened first?  The
> NVIDIA driver wouldn't know about this, but it would know when the
> first NVIDIA device is opened and be able to establish p2p for the
> NVIDIA devices at that point.  Can we do what we need with that model?
> What if libvirt is asked to hot-add an NVIDIA vGPU?  It would need to
> do a create on the NVIDIA parent device with the existing group id, at
> which point the NVIDIA vendor driver could fail the device create if
> the p2p setup has already been done.  The Intel vendor driver might
> allow it.  Similar to open, the last close of the mdev device for a
> given vendor (which might not be the last close of mdev devices within
> the group) would need to trigger the offline process for that vendor.
> 
> That all sounds well and good... here's the kicker: iommu groups
> necessarily need to be part of the same iommu context, ie.
> vfio container.  How do we deal with vIOMMUs within the guest when we
> are intentionally forcing a set of devices within the same context?
> This is why it's _very_ beneficial on the host to create iommu groups
> with the smallest number of devices we can reasonably trust to be
> isolated.  We're backing ourselves into a corner if we tell libvirt
> that the standard process is to put all mdev devices into a single
> group.  The grouping/startup issue is still unresolved in my head.
> Thanks,
> 
> Alex
> 

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

Re: [libvirt] [Qemu-devel] [PATCH v7 0/4] Add Mediated device support

[Alex Williamson]

On Sat, 3 Sep 2016 22:01:13 +0530
Kirti Wankhede  wrote:

> On 9/3/2016 1:59 AM, John Ferlan wrote:
> > 
> > 
> > On 09/02/2016 02:33 PM, Kirti Wankhede wrote:  
> >>
> >> On 9/2/2016 10:55 PM, Paolo Bonzini wrote:  
> >>>
> >>>
> >>> On 02/09/2016 19:15, Kirti Wankhede wrote:  
>  On 9/2/2016 3:35 PM, Paolo Bonzini wrote:  
> >
> >  my-vgpu
> >  pci__86_00_0
> >  
> >
> >0695d332-7831-493f-9e71-1c85c8911a08
> >  
> >
> >
> > After creating the vGPU, if required by the host driver, all the other
> > type ids would disappear from "virsh nodedev-dumpxml pci__86_00_0" 
> > too.  
> 
>  Thanks Paolo for details.
>  'nodedev-create' parse the xml file and accordingly write to 'create'
>  file in sysfs to create mdev device. Right?
>  At this moment, does libvirt know which VM this device would be
>  associated with?  
> >>>
> >>> No, the VM will associate to the nodedev through the UUID.  The nodedev
> >>> is created separately from the VM.
> >>>  
> > When dumping the mdev with nodedev-dumpxml, it could show more complete
> > info, again taken from sysfs:
> >
> >
> >  my-vgpu
> >  pci__86_00_0
> >  
> >0695d332-7831-493f-9e71-1c85c8911a08
> >
> >
> >  
> >
> >
> >  
> >  
> >  ...
> >  NVIDIA
> >
> >  
> >
> >
> > Notice how the parent has mdev inside pci; the vGPU, if it has to have
> > pci at all, would have it inside mdev.  This represents the difference
> > between the mdev provider and the mdev device.  
> 
>  Parent of mdev device might not always be a PCI device. I think we
>  shouldn't consider it as PCI capability.  
> >>>
> >>> The  in the vGPU means that it _will_ be exposed
> >>> as a PCI device by VFIO.
> >>>
> >>> The  in the physical GPU means that the GPU is a
> >>> PCI device.
> >>>  
> >>
> >> Ok. Got that.
> >>  
> > Random proposal for the domain XML too:
> >
> >   
> > 
> >   
> >   0695d332-7831-493f-9e71-1c85c8911a08
> > 
> > 
> >   
> >  
> 
>  When user wants to assign two mdev devices to one VM, user have to add
>  such two entries or group the two devices in one entry?  
> >>>
> >>> Two entries, one per UUID, each with its own PCI address in the guest.
> >>>  
>  On other mail thread with same subject we are thinking of creating group
>  of mdev devices to assign multiple mdev devices to one VM.  
> >>>
> >>> What is the advantage in managing mdev groups?  (Sorry didn't follow the
> >>> other thread).
> >>>  
> >>
> >> When mdev device is created, resources from physical device is assigned
> >> to this device. But resources are committed only when device goes
> >> 'online' ('start' in v6 patch)
> >> In case of multiple vGPUs in a VM for Nvidia vGPU solution, resources
> >> for all vGPU devices in a VM are committed at one place. So we need to
> >> know the vGPUs assigned to a VM before QEMU starts.
> >>
> >> Grouping would help here as Alex suggested in that mail. Pulling only
> >> that part of discussion here:
> >>
> >>  It seems then that the grouping needs to affect the iommu group
> >> so that  
> >>> you know that there's only a single owner for all the mdev devices
> >>> within the group.  IIRC, the bus drivers don't have any visibility
> >>> to opening and releasing of the group itself to trigger the
> >>> online/offline, but they can track opening of the device file
> >>> descriptors within the group.  Within the VFIO API the user cannot
> >>> access the device without the device file descriptor, so a "first
> >>> device opened" and "last device closed" trigger would provide the
> >>> trigger points you need.  Some sort of new sysfs interface would need
> >>> to be invented to allow this sort of manipulation.
> >>> Also we should probably keep sight of whether we feel this is
> >>> sufficiently necessary for the complexity.  If we can get by with only
> >>> doing this grouping at creation time then we could define the "create"
> >>> interface in various ways.  For example:
> >>>
> >>> echo $UUID0 > create
> >>>
> >>> would create a single mdev named $UUID0 in it's own group.
> >>>
> >>> echo {$UUID0,$UUID1} > create
> >>>
> >>> could create mdev devices $UUID0 and $UUID1 grouped together.
> >>>  
> >> 
> >>
> >> 
> >> I think this would create mdev device of same type on same parent
> >> device. We need to consider the case of multiple mdev devices of
> >> different types and with different parents to be grouped together.
> >> 
> >>
> >>  We could even do:  
> >>>
> >>> echo $UUID1:$GROUPA > create
> >>>
> >>> where $GROUPA is the group ID of a previously created mdev device into
> >>> which $UUID1 is to be created and added to the 

Re: [libvirt] [Qemu-devel] [PATCH v7 0/4] Add Mediated device support

[Alex Williamson]

On Sat, 3 Sep 2016 22:04:56 +0530
Kirti Wankhede  wrote:

> On 9/3/2016 3:18 AM, Paolo Bonzini wrote:
> > 
> > 
> > On 02/09/2016 20:33, Kirti Wankhede wrote:  
> >>  We could even do:  
> 
>  echo $UUID1:$GROUPA > create
> 
>  where $GROUPA is the group ID of a previously created mdev device into
>  which $UUID1 is to be created and added to the same group.  
> >>   
> > 
> > From the point of view of libvirt, I think I prefer Alex's idea.
> >  could be an additional element in the nodedev-create XML:
> > 
> > 
> >   my-vgpu
> >   pci__86_00_0
> >   
> > 
> > 0695d332-7831-493f-9e71-1c85c8911a08
> > group1
> >   
> > 
> > 
> > (should group also be a UUID?)
> >   
> 
> No, this should be a unique number in a system, similar to iommu_group.

Sorry, just trying to catch up on this thread after a long weekend.

We're talking about iommu groups here, we're not creating any sort of
parallel grouping specific to mdev devices.  This is why my example
created a device and then required the user to go find the group number
given to that device in order to create another device within the same
group.  iommu group numbering is not within the user's control and is
not a uuid.  libvirt can refer to the group as anything it wants in the
xml, but the host group number is allocated by the host, not under user
control, is not persistent.  libvirt would just be giving it a name to
know which devices are part of the same group.  Perhaps the runtime xml
would fill in the group number once created.

There were also a lot of unanswered questions in my proposal, it's not
clear that there's a standard algorithm for when mdev devices need to
be grouped together.  Should we even allow groups to span multiple host
devices?  Should they be allowed to span devices from different
vendors?

If we imagine a scenario of a group composed of a mix of Intel and
NVIDIA vGPUs, what happens when an Intel device is opened first?  The
NVIDIA driver wouldn't know about this, but it would know when the
first NVIDIA device is opened and be able to establish p2p for the
NVIDIA devices at that point.  Can we do what we need with that model?
What if libvirt is asked to hot-add an NVIDIA vGPU?  It would need to
do a create on the NVIDIA parent device with the existing group id, at
which point the NVIDIA vendor driver could fail the device create if
the p2p setup has already been done.  The Intel vendor driver might
allow it.  Similar to open, the last close of the mdev device for a
given vendor (which might not be the last close of mdev devices within
the group) would need to trigger the offline process for that vendor.

That all sounds well and good... here's the kicker: iommu groups
necessarily need to be part of the same iommu context, ie.
vfio container.  How do we deal with vIOMMUs within the guest when we
are intentionally forcing a set of devices within the same context?
This is why it's _very_ beneficial on the host to create iommu groups
with the smallest number of devices we can reasonably trust to be
isolated.  We're backing ourselves into a corner if we tell libvirt
that the standard process is to put all mdev devices into a single
group.  The grouping/startup issue is still unresolved in my head.
Thanks,

Alex

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

Re: [libvirt] [libvirt-php] libvirt_connect not reading out credential info on 0.5.2

[Fernando Casas Schössow]

Thanks for the explanation Michal.

I will be looking forward the fix to try it. :)

On mar, sep 6, 2016 at 6:59 , Michal Privoznik  
wrote:

On 06.09.2016 13:45, Fernando Casas Schössow wrote:
 Thanks for the instructions since I'm not familiar with debugging 
as you

 probably guessed. :)

 Here you have gdb output:

 GNU gdb (GDB) Red Hat Enterprise Linux 7.6.1-80.el7
 Copyright (C) 2013 Free Software Foundation, Inc.
 License GPLv3+: GNU GPL version 3 or later
 
 This is free software: you are free to change and redistribute it.
 There is NO WARRANTY, to the extent permitted by law. Type "show 
copying"

 and "show warranty" for details.
 This GDB was configured as "x86_64-redhat-linux-gnu".
 For bug reporting instructions, please see:
 ...
 Reading symbols from /usr/bin/php...Reading symbols from
 /usr/bin/php...(no debugging symbols found)...done.
 (no debugging symbols found)...done.
 Missing separate debuginfos, use: debuginfo-install
 php-cli-5.4.16-36.3.el7_2.x86_64
 (gdb) run /storage/lighttpd/wwwroot/libvirt.php
 Starting program: /usr/bin/php /storage/lighttpd/wwwroot/libvirt.php
 [Thread debugging using libthread_db enabled]
 Using host libthread_db library "/lib64/libthread_db.so.1".
 [New Thread 0x7fffe2ceb700 (LWP 31799)]
 [Thread 0x7fffe2ceb700 (LWP 31799) exited]
 PHP Warning: unlink(test.log): No such file or directory in
 /storage/lighttpd/wwwroot/libvirt.php on line 4

 Program received signal SIGSEGV, Segmentation fault.
 0x7fffe9ad54a0 in virClassIsDerivedFrom () from 
/lib64/libvirt.so.0

 (gdb) backtrace
 #0 0x7fffe9ad54a0 in virClassIsDerivedFrom () from 
/lib64/libvirt.so.0

 #1 0x7fffe9b861ac in virDomainLookupByUUIDString () from
 /lib64/libvirt.so.0
 #2 0x7fffea1e82c5 in generate_uuid () from
 /usr/lib64/php/modules/libvirt-php.so
 #3 0x7fffe7d3385a in wsmc_create_request () from
 /lib64/libwsman_client.so.4
 #4 0x7fffe7d3546f in wsmc_action_enumerate () from
 /lib64/libwsman_client.so.4
 #5 0x7fffe9c8e34a in hypervEnumAndPull () from 
/lib64/libvirt.so.0

 #6 0x7fffe9c8ee49 in hypervGetMsvmComputerSystemList () from
 /lib64/libvirt.so.0
 #7 0x7fffe9c8dacd in hypervConnectOpen () from 
/lib64/libvirt.so.0
 #8 0x7fffe9b82f39 in virConnectOpenInternal () from 
/lib64/libvirt.so.0
 #9 0x7fffe9b845ce in virConnectOpenAuth () from 
/lib64/libvirt.so.0

 #10 0x7fffea1e1190 in zif_libvirt_connect () from
 /usr/lib64/php/modules/libvirt-php.so
 #11 0x5586b57c in zend_do_fcall_common_helper_SPEC ()
 #12 0x557e8527 in execute ()
 #13 0x557c115f in zend_execute_scripts ()
 #14 0x55760976 in php_execute_script ()
 #15 0x5586d598 in do_cli ()
 #16 0x5561a12e in main ()



Ah, this is exactly what I needed. Well, we're doomed. There are two
problems here.

The first one is: libvirt-php is exporting symbols that is shouldn't
have. Like for instance generate_uuid which is clearly meant as an
internal helper but because of our own bug (well, missing
implementation) it is being exported. Thus, when linker is linking the
libraries together it finds out that libwsman_client.so.4 needs
generate_uuid symbol which it finds in libvirt-pgp.so instead of
libwsman.so.1.0.0 (which is what libwsman devels intended).

objdump -T /usr/lib64/libwsman_client.so.4 | grep generate_uuid
  D  *UND*    
generate_uuid


bjdump -T /usr/lib64/php/modules/libvirt-php.so | grep generate
0002b660 gDF .text  00ce  Base
generate_uuid
0002b590 gDF .text  00cb  Base
generate_uuid_any


objdump -T /lib64/libwsman.so.1.0.0 | grep generate
e820 gDF .text  01bd  Base
generate_uuid
0001f950 gDF .text  005a  Base
wsman_generate_fault_buffer
0001f8c0 gDF .text  0082  Base
wsman_generate_fault


The fix on our part is obvious.

The second problem is, libwsman devels should not really be using such
generic names in their libraries. At least they need to prefix their
public function names with wsman_ (see those other 'generate_fault'
functions?).

I'll work on fix and let you test it - since the backtrace proof this
is hyperv specific bug. That's why I was unable to reproduce it.

Thanks for the report!

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

Re: [libvirt] [libvirt-php] libvirt_connect not reading out credential info on 0.5.2

[Michal Privoznik]

On 06.09.2016 13:45, Fernando Casas Schössow wrote:
> Thanks for the instructions since I'm not familiar with debugging as you
> probably guessed. :)
> 
> Here you have gdb output:
> 
> GNU gdb (GDB) Red Hat Enterprise Linux 7.6.1-80.el7
> Copyright (C) 2013 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later
> 
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law. Type "show copying"
> and "show warranty" for details.
> This GDB was configured as "x86_64-redhat-linux-gnu".
> For bug reporting instructions, please see:
> ...
> Reading symbols from /usr/bin/php...Reading symbols from
> /usr/bin/php...(no debugging symbols found)...done.
> (no debugging symbols found)...done.
> Missing separate debuginfos, use: debuginfo-install
> php-cli-5.4.16-36.3.el7_2.x86_64
> (gdb) run /storage/lighttpd/wwwroot/libvirt.php
> Starting program: /usr/bin/php /storage/lighttpd/wwwroot/libvirt.php
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/lib64/libthread_db.so.1".
> [New Thread 0x7fffe2ceb700 (LWP 31799)]
> [Thread 0x7fffe2ceb700 (LWP 31799) exited]
> PHP Warning: unlink(test.log): No such file or directory in
> /storage/lighttpd/wwwroot/libvirt.php on line 4
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x7fffe9ad54a0 in virClassIsDerivedFrom () from /lib64/libvirt.so.0
> (gdb) backtrace
> #0 0x7fffe9ad54a0 in virClassIsDerivedFrom () from /lib64/libvirt.so.0
> #1 0x7fffe9b861ac in virDomainLookupByUUIDString () from
> /lib64/libvirt.so.0
> #2 0x7fffea1e82c5 in generate_uuid () from
> /usr/lib64/php/modules/libvirt-php.so
> #3 0x7fffe7d3385a in wsmc_create_request () from
> /lib64/libwsman_client.so.4
> #4 0x7fffe7d3546f in wsmc_action_enumerate () from
> /lib64/libwsman_client.so.4
> #5 0x7fffe9c8e34a in hypervEnumAndPull () from /lib64/libvirt.so.0
> #6 0x7fffe9c8ee49 in hypervGetMsvmComputerSystemList () from
> /lib64/libvirt.so.0
> #7 0x7fffe9c8dacd in hypervConnectOpen () from /lib64/libvirt.so.0
> #8 0x7fffe9b82f39 in virConnectOpenInternal () from /lib64/libvirt.so.0
> #9 0x7fffe9b845ce in virConnectOpenAuth () from /lib64/libvirt.so.0
> #10 0x7fffea1e1190 in zif_libvirt_connect () from
> /usr/lib64/php/modules/libvirt-php.so
> #11 0x5586b57c in zend_do_fcall_common_helper_SPEC ()
> #12 0x557e8527 in execute ()
> #13 0x557c115f in zend_execute_scripts ()
> #14 0x55760976 in php_execute_script ()
> #15 0x5586d598 in do_cli ()
> #16 0x5561a12e in main ()


Ah, this is exactly what I needed. Well, we're doomed. There are two
problems here.

The first one is: libvirt-php is exporting symbols that is shouldn't
have. Like for instance generate_uuid which is clearly meant as an
internal helper but because of our own bug (well, missing
implementation) it is being exported. Thus, when linker is linking the 
libraries together it finds out that libwsman_client.so.4 needs
generate_uuid symbol which it finds in libvirt-pgp.so instead of
libwsman.so.1.0.0 (which is what libwsman devels intended).

objdump -T /usr/lib64/libwsman_client.so.4 | grep generate_uuid
  D  *UND*    generate_uuid

bjdump -T /usr/lib64/php/modules/libvirt-php.so | grep generate
0002b660 gDF .text  00ce  Basegenerate_uuid
0002b590 gDF .text  00cb  Basegenerate_uuid_any

objdump -T /lib64/libwsman.so.1.0.0 | grep generate
e820 gDF .text  01bd  Basegenerate_uuid
0001f950 gDF .text  005a  Base
wsman_generate_fault_buffer
0001f8c0 gDF .text  0082  Base
wsman_generate_fault

The fix on our part is obvious.

The second problem is, libwsman devels should not really be using such
generic names in their libraries. At least they need to prefix their
public function names with wsman_ (see those other 'generate_fault'
functions?).

I'll work on fix and let you test it - since the backtrace proof this
is hyperv specific bug. That's why I was unable to reproduce it.

Thanks for the report!

Michal

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

Re: [libvirt] [PATCH] Make sure sys/types.h is included after sys/sysmacros.h

[Eric Blake]

On 09/06/2016 08:46 AM, Michal Privoznik wrote:
> In the latest glibc, major() and minor() functions are marked as
> deprecated (glibc commit dbab6577):

Not only that, but there's a thread on the autoconf list about how to
fix AC_HEADER_MAJOR to work.  I may be releasing autoconf 2.70 in the
near future because of this glibc change, although there is still the
issue that it will only benefit programs that are configured with
new-enough autoconf with the fixed macro.  Gnulib, of course, will
backport any autoconf fix, and we'll at least pick it up via gnulib if
we update the submodule, once everything settles upstream in autoconf,
but it may be a few days before I know how best to tackle it among all
the related projects.

> 
> This still won't solve the build issue completely as AC_HEADER_MAJOR still
> reports that major() is defined by sys/types.h instead of sys/sysmacros.h.
> But once they fix it, we are good too. Or we can use the following
> workaround in configure.ac:
> 
>   +old_CFLAGS=$CFLAGS
>   +CFLAGS+=" -Werror "
>AC_HEADER_MAJOR
>   +CFLAGS=$old_CFLAGS
> 

I don't think we need to worry about working around it in our
configure.ac; once upstream autoconf and gnulib figure out what they
want to do, we can inherit that.

> +++ b/src/conf/domain_audit.c
> @@ -24,7 +24,6 @@
>  #include 
>  
>  #include 
> -#include 
>  
>  #ifdef MAJOR_IN_MKDEV
>  # include 
> @@ -32,6 +31,8 @@
>  # include 
>  #endif
>  
> +#include 
> +

For the purposes of immediate compilation, this looks correct to me, so
you have my ACK.


-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



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

Re: [libvirt] [PATCH] tools: Pass opaque data in vshCompleter and introduce autoCompleteOpaque

[Michal Privoznik]

On 06.09.2016 14:04, Nishith Shah wrote:
> This patch changes the signature of vshCompleters, allowing to pass along
> some data that we might want to along with the completers; for example,
> we might want to pass the autocomplete vshControl along with the
> completer, in case the completer requires a connection to libvirtd.
> 
> Signed-off-by: Nishith Shah 
> ---
>  tools/vsh.c | 10 +-
>  tools/vsh.h |  2 +-
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/vsh.c b/tools/vsh.c
> index 3157922..3cc03ec 100644
> --- a/tools/vsh.c
> +++ b/tools/vsh.c
> @@ -64,6 +64,11 @@
>  # define SA_SIGINFO 0
>  #endif
>  
> +#ifdef WITH_READLINE
> +/* For autocompletion */
> +void *autoCompleteOpaque;
> +#endif
> +
>  /* NOTE: It would be much nicer to have these two as part of vshControl
>   * structure, unfortunately readline doesn't support passing opaque data
>   * and only relies on static data accessible from the user-side callback
> @@ -2808,7 +2813,8 @@ vshReadlineParse(const char *text, int state)
>  res = vshReadlineOptionsGenerator(sanitized_text, state, cmd);
>  } else if (non_bool_opt_exists && data_complete && opt->completer) {
>  if (!completed_list)
> -completed_list = opt->completer(opt->completer_flags);
> +completed_list = opt->completer(autoCompleteOpaque,
> +opt->completer_flags);
>  if (completed_list) {
>  while ((completed_name = completed_list[completed_list_index])) {
>  completed_list_index++;
> @@ -2858,6 +2864,8 @@ vshReadlineInit(vshControl *ctl)
>  char *histsize_env = NULL;
>  const char *histsize_str = NULL;
>  
> +/* Opaque data for autocomplete callbacks and connections to libvirtd */
> +autoCompleteOpaque = ctl;

I'd put a empty space here so that it creates a visual barrier before
next comment starts.

>  /* Allow conditional parsing of the ~/.inputrc file.
>   * Work around ancient readline 4.1 (hello Mac OS X),
>   * which declared it as 'char *' instead of 'const char *'.
> diff --git a/tools/vsh.h b/tools/vsh.h
> index 7f43055..e53910f 100644
> --- a/tools/vsh.h
> +++ b/tools/vsh.h
> @@ -123,7 +123,7 @@ typedef struct _vshCmdOpt vshCmdOpt;
>  typedef struct _vshCmdOptDef vshCmdOptDef;
>  typedef struct _vshControl vshControl;
>  
> -typedef char **(*vshCompleter)(unsigned int flags);
> +typedef char **(*vshCompleter)(void *opaque, unsigned int flags);
>  
>  /*
>   * vshCmdInfo -- name/value pair for information about command
> 

ACKed and pushed.

Michal

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

Re: [libvirt] [PATCH] qemu: guest agent: introduce new error code VIR_ERR_AGENT_UNSYNCED

[Yuriy Pudgorodskiy]

On 9/6/2016 6:03 PM, Michal Privoznik wrote:

On 05.09.2016 18:42, Maxim Nestratov wrote:

From: Yuri Pudgorodskiy 

A separate error code will help recognize real failures from
necessity to try again

Signed-off-by: Maxim Nestratov 
---
  include/libvirt/virterror.h | 2 ++
  src/qemu/qemu_agent.c   | 6 +++---
  src/util/virerror.c | 6 ++
  3 files changed, 11 insertions(+), 3 deletions(-)

ACK

Just curious - have you experienced this behaviour?

Michal
.

Issue any GA command while  domains is just booted gives a good chance 
of the following:


- domain is started running
- virtio agent.0 channel is already connected to guest, but qemu-ga is 
not ready yet

- a libvirt client issues any GA commands
- command begins with guest-sync written to virtio serial device buffer
- qemu-ga did not respond within default timeout (5 seconds), client got 
TIMEOUT
- a client repeated GA commands, waiting for GA to come up, buffer is 
populated with repeated guest-sync command
- GA finally reads command from virtio, responding to id from the 1st 
guest-sync found in channel (FIFO)

- but libvirt now expects last id, and responds with VIR_ERR_INTERNAL_ERROR
- ... VIR_ERR_INTERNAL_ERROR looks terrible from the client point of view

Moreover, any timeout from qemu-ga results in VIR_ERR_AGENT_UNSYNCED error.
Same scenario:
- two guest agent commands one after another
- 1st timeouted
- libvirt is expecting response with id of the 2nd
- qemu-ga finally comes with answer to the 1st
- libvirt is under if (id_ret != id) condition and is responding with 
VIR_ERR_AGENT_UNSYNCED


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

Re: [libvirt] [PATCH] util: hostcpu: improve CPU freq code for FreeBSD

[Roman Bogorodskiy]

  Michal Privoznik wrote:

> On 31.08.2016 07:14, Roman Bogorodskiy wrote:
> > Current implementation uses the dev.cpu.0.freq sysctl that is
> > provided by the cpufreq(4) framework and returns the actual
> > CPU frequency. However, there are environment where it's not available,
> > e.g. when running nested in KVM. In this case fall back to hw.clockrate
> > that reports CPU frequency at the boot time.
> > 
> > Resolves (hopefully):
> > https://bugzilla.redhat.com/show_bug.cgi?id=1369964
> > ---
> >  src/util/virhostcpu.c | 11 +--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
> > index 0f03ff8..b5a37a8 100644
> > --- a/src/util/virhostcpu.c
> > +++ b/src/util/virhostcpu.c
> 
> ACK

Pushed, thanks!

> Michal

Roman Bogorodskiy


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

Re: [libvirt] [PATCH] qemu: guest agent: introduce new error code VIR_ERR_AGENT_UNSYNCED

[Maxim Nestratov]

06-Sep-16 18:03, Michal Privoznik пишет:


On 05.09.2016 18:42, Maxim Nestratov wrote:

From: Yuri Pudgorodskiy 

A separate error code will help recognize real failures from
necessity to try again

Signed-off-by: Maxim Nestratov 
---
  include/libvirt/virterror.h | 2 ++
  src/qemu/qemu_agent.c   | 6 +++---
  src/util/virerror.c | 6 ++
  3 files changed, 11 insertions(+), 3 deletions(-)

ACK

Just curious - have you experienced this behaviour?

Michal


Yeah, pretty often.

Maxim

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

Re: [libvirt] [PATCH] qemu: guest agent: introduce new error code VIR_ERR_AGENT_UNSYNCED

[Michal Privoznik]

On 05.09.2016 18:42, Maxim Nestratov wrote:
> From: Yuri Pudgorodskiy 
> 
> A separate error code will help recognize real failures from
> necessity to try again
> 
> Signed-off-by: Maxim Nestratov 
> ---
>  include/libvirt/virterror.h | 2 ++
>  src/qemu/qemu_agent.c   | 6 +++---
>  src/util/virerror.c | 6 ++
>  3 files changed, 11 insertions(+), 3 deletions(-)

ACK

Just curious - have you experienced this behaviour?

Michal

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

Re: [libvirt] [PATCH] util: fix crash in virClassIsDerivedFrom for CloseCallbacks objects

[Michal Privoznik]

On 05.09.2016 18:42, Maxim Nestratov wrote:
> There is a possibility that qemu driver frees by unreferencing its
> closeCallbacks pointer as it has the only reference to the object,
> while in fact not all users of CloseCallbacks called thier
> virCloseCallbacksUnset.
> 
> Backtrace is the following:
> Thread #1:
> 0  in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0
> 1  in virCondWait (c=, m=)
> at util/virthread.c:154
> 2  in virThreadPoolFree (pool=0x7f0810110b50)
> at util/virthreadpool.c:266
> 3  in qemuStateCleanup () at qemu/qemu_driver.c:1116
> 4  in virStateCleanup () at libvirt.c:808
> 5  in main (argc=, argv=)
> at libvirtd.c:1660
> 
> Thread #2:
> 0  in virClassIsDerivedFrom (klass=0xdeadbeef, parent=0x7f0837c694d0) at 
> util/virobject.c:169
> 1  in virObjectIsClass (anyobj=anyobj@entry=0x7f08101d4760, klass= out>) at util/virobject.c:365
> 2  in virObjectLock (anyobj=0x7f08101d4760) at util/virobject.c:317
> 3  in virCloseCallbacksUnset (closeCallbacks=0x7f08101d4760, 
> vm=vm@entry=0x7f08101d47b0, cb=cb@entry=0x7f081d078fc0 
> ) at util/virclosecallbacks.c:163
> 4  in qemuProcessAutoDestroyRemove (driver=driver@entry=0x7f081018be50, 
> vm=vm@entry=0x7f08101d47b0) at qemu/qemu_process.c:6368
> 5  in qemuProcessStop (driver=driver@entry=0x7f081018be50, 
> vm=vm@entry=0x7f08101d47b0, reason=reason@entry=VIR_DOMAIN_SHUTOFF_SHUTDOWN, 
> asyncJob=asyncJob@entry=QEMU_ASYNC_JOB_NONE, flags=flags@entry=0) at 
> qemu/qemu_process.c:5854
> 6  in processMonitorEOFEvent (vm=0x7f08101d47b0, driver=0x7f081018be50) at 
> qemu/qemu_driver.c:4585
> 7  qemuProcessEventHandler (data=, opaque=0x7f081018be50) at 
> qemu/qemu_driver.c:4629
> 8  in virThreadPoolWorker (opaque=opaque@entry=0x7f0837c4f820) at 
> util/virthreadpool.c:145
> 9  in virThreadHelper (data=) at util/virthread.c:206
> 10 in start_thread () from /lib64/libpthread.so.0
> 
> Let's reference CloseCallbacks object in virCloseCallbacksSet and
> unreference in virCloseCallbacksUnset.
> 
> Signed-off-by: Maxim Nestratov 
> ---
>  src/util/virclosecallbacks.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/util/virclosecallbacks.c b/src/util/virclosecallbacks.c
> index 82d4071..891a92b 100644
> --- a/src/util/virclosecallbacks.c
> +++ b/src/util/virclosecallbacks.c
> @@ -141,6 +141,7 @@ virCloseCallbacksSet(virCloseCallbacksPtr closeCallbacks,
>  virObjectRef(vm);
>  }
>  
> +virObjectRef(closeCallbacks);
>  ret = 0;
>   cleanup:
>  virObjectUnlock(closeCallbacks);
> @@ -180,6 +181,8 @@ virCloseCallbacksUnset(virCloseCallbacksPtr 
> closeCallbacks,
>  ret = 0;
>   cleanup:
>  virObjectUnlock(closeCallbacks);
> +if (!ret)
> +virObjectUnref(closeCallbacks);

Might as well put this a few lines above, just before 'ret = 0' line.

ACK with that changed.

Michal

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

Re: [libvirt] [PATCH] virsh: Fix *-event error string

[Christophe Fergeau]

On Tue, Sep 06, 2016 at 03:07:37PM +0200, Erik Skultety wrote:
> ACK, although, I have a really small nitpick the decision on fixing of
> which I leave to you. After the fix, "--list or --event type...", at
> least to me, sounds like "--event" was the mentioned type itself which
> is of course wrong. Maybe it could be better (just an idea) to use angle
> brackets like this '' to make the message more clear, but as I
> said, your call.

I considered doing it, but did not for no particular reason. I'll change
it before pushing.

Thanks,

Christophe


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

Re: [libvirt] [PATCH 0/4] systemd-related fixes and improvements

[Andrea Bolognani]

On Tue, 2016-09-06 at 14:58 +0100, Daniel P. Berrange wrote:
> On Tue, Sep 06, 2016 at 03:55:20PM +0200, Andrea Bolognani wrote:
> > 
> > Make libvirt on systemd nicer for the user, by getting rid of
> > some confusing behavior, and overall more solid.
> > 
> > More details in each specific patch.
> 
> ACK to all

Pushed, thanks :)

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [Qemu-devel] qapi DEVICE_DELETED event issued *before* instance_finalize?!

[Paolo Bonzini]

On 06/09/2016 16:02, Laine Stump wrote:
>>>
>> It seems like this is just pointing out another flaw in the semantics
>> of DEVICE_DELETED, a device can linger without a device id, so there's
>> no way to reference it via QMP.
> 
> Ah, right. I hadn't caught that. Yeah, since it's the device id that's
> used to keep track of which device the event is for, then it seems
> impossible to have an event that's issued after the device id is already
> recycled.

If a device lingers for more than say a second (most likely less---what
you're looking for is one or two synchronize_rcu cycles), it would be a
bug in QEMU.

Paolo

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

Re: [libvirt] [PATCH] util: hostcpu: improve CPU freq code for FreeBSD

[Michal Privoznik]

On 31.08.2016 07:14, Roman Bogorodskiy wrote:
> Current implementation uses the dev.cpu.0.freq sysctl that is
> provided by the cpufreq(4) framework and returns the actual
> CPU frequency. However, there are environment where it's not available,
> e.g. when running nested in KVM. In this case fall back to hw.clockrate
> that reports CPU frequency at the boot time.
> 
> Resolves (hopefully):
> https://bugzilla.redhat.com/show_bug.cgi?id=1369964
> ---
>  src/util/virhostcpu.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c
> index 0f03ff8..b5a37a8 100644
> --- a/src/util/virhostcpu.c
> +++ b/src/util/virhostcpu.c
> @@ -994,9 +994,16 @@ virHostCPUGetInfo(virArch hostarch ATTRIBUTE_UNUSED,
>  *threads = 1;
>  
>  # ifdef __FreeBSD__
> +/* dev.cpu.%d.freq reports current active CPU frequency. It is provided 
> by
> + * the cpufreq(4) framework. However, it might be disabled or no driver
> + * available. In this case fallback to "hw.clockrate" which reports boot 
> time
> + * CPU frequency. */
> +
>  if (sysctlbyname("dev.cpu.0.freq", _freq, _freq_len, NULL, 0) < 
> 0) {
> -virReportSystemError(errno, "%s", _("cannot obtain CPU freq"));
> -return -1;
> +if (sysctlbyname("hw.clockrate", _freq, _freq_len, NULL, 0) 
> < 0) {
> +virReportSystemError(errno, "%s", _("cannot obtain CPU freq"));
> +return -1;
> +}
>  }
>  
>  *mhz = cpu_freq;
> 

ACK

Michal

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

Re: [libvirt] [Qemu-devel] qapi DEVICE_DELETED event issued *before* instance_finalize?!

[Paolo Bonzini]

On 06/09/2016 04:18, Alex Williamson wrote:
> On Mon, 5 Sep 2016 11:36:55 +0200
> Paolo Bonzini  wrote:
>> DEVICE_DELETED does have a meaning: management cannot talk to the device
>> anymore in QMP once it is raised.
> 
> It seems like this is just pointing out another flaw in the semantics
> of DEVICE_DELETED, a device can linger without a device id, so there's
> no way to reference it via QMP.  QEMU can't signal anything more about
> the device, nor can the VM admin perform any further operations on it.
> It's like detecting planets around distant stars, libvirt can't actually
> see the device, it can only monitor the affects the device has on the
> VM.  This is broken and it seems like the fix is to push both the
> release of the device id and the DEVICE_DELETED notification until
> after the instance_finalize callback.

You can't do that.  Think of it as DEVICE_DELETED being "removal" and
instance_finalize being "reference count has gone to 0".  You cannot
make the reference count go to 0 unless you have disconnected the device
from the parent, and the parent is the one that remembers the device id.

>> Technically what libvirt wants to know for VFIO is not whether the
>> device is gone; it's whether the device's _backend_ (the VFIO file
>> descriptor) is gone.  The device backend could have been a separate QOM
>> object, but it isn't.
>>
>> So perhaps we need a new event that is specific to VFIO?
> 
> This immediately sounds like the wrong path.  A) Why is this vfio
> specific?

Because VFIO doesn't have a separate backend object.  instance_finalize
is already where the backends are released.  You cannot for example
reuse a character device until instance_finalize (no event is generated,
but we could certainly add one to qemu-char.c if deemed useful).
VFIO_DELETED is just another example of the same thing.

It just happens that the host device is not a separate "-object
vfio-backend-pci,sysfs=..." but it's embedded in "-device vfio-pci" so
the code for the new event must go in hw/vfio rather than a hypothetical
backends/vfio.

I'm not saying that VFIO should have a separate backend object, that
would probably be overengineering.  But in some cases you get saner
semantics if you think of VFIO as a composition of two things.

> B) Without a device id, how are we going to signal an
> event?

For example by sysfs path or host device path---it makes sense to use
properties of the backend since this signals that the backend is now free.

Paolo

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

Re: [libvirt] [Qemu-devel] qapi DEVICE_DELETED event issued *before* instance_finalize?!

[Laine Stump]

On 09/05/2016 10:18 PM, Alex Williamson wrote:

On Mon, 5 Sep 2016 11:36:55 +0200
Paolo Bonzini  wrote:


On 05/09/2016 11:23, Markus Armbruster wrote:

On the other hand, it is clearly documented that the DEVICE_DELETED
event is sent as soon as guest acknowledges completion of device
removal. So libvirt's buggy if we'd follow documentation strictly. But
then again, I don't see much information value in "guest has detached
device but qemu hasn't yet" event. Libvirt would ignore such event.

Unless I'm missing something, libvirt needs an event that signals "Guest
and QEMU are done with this device".  Current DEVICE_DELETED isn't.

Can we imagine a use for current DEVICE_DELETED, i.e. "Guest is done,
but QEMU isn't"?

Would anything break if we changed semantics of DEVICE_DELETED to what
libvirt actually needs?

If the answers are "no" and "no", let's do it.

There is a subtle aspect of this.  After the current DEVICE_DELETED, the
device id is not used any more.  So technically you could have

device_add bar,id=foo
device_del foo

// something in QEMU prevents the device from going away?
// for example there is a storage issue that blocks completion
// of a read(), and bar is a storage device

device_add bar,id=foo
device_del foo

// which foo is being deleted?  The old one or the new one?
event DEVICE_DELETED

DEVICE_DELETED does have a meaning: management cannot talk to the device
anymore in QMP once it is raised.

It seems like this is just pointing out another flaw in the semantics
of DEVICE_DELETED, a device can linger without a device id, so there's
no way to reference it via QMP.


Ah, right. I hadn't caught that. Yeah, since it's the device id that's 
used to keep track of which device the event is for, then it seems 
impossible to have an event that's issued after the device id is already 
recycled.



  QEMU can't signal anything more about
the device, nor can the VM admin perform any further operations on it.
It's like detecting planets around distant stars, libvirt can't actually
see the device, it can only monitor the affects the device has on the
VM.  This is broken and it seems like the fix is to push both the
release of the device id and the DEVICE_DELETED notification until
after the instance_finalize callback.  Doesn't that solve the nuance
you've identified here as well?


This works perfectly for libvirt.

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

[libvirt] [PATCH 4/4] libvirt-guests.service: Add Requires=libvirtd.service

[Andrea Bolognani]

Having After=libvirtd.service merely ensures that, if both
services are asked to start, libvirtd.service will start
first.

What we really want is for libvirtd.service to be started
whenever libvirt-guests.service is asked to start. Adding a
Requires= relationship guarantees that will happen.
---
 tools/libvirt-guests.service.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/libvirt-guests.service.in b/tools/libvirt-guests.service.in
index 3c901b9..b4f54f2 100644
--- a/tools/libvirt-guests.service.in
+++ b/tools/libvirt-guests.service.in
@@ -1,5 +1,6 @@
 [Unit]
 Description=Suspend/Resume Running libvirt Guests
+Requires=libvirtd.service
 After=network.target
 After=time-sync.target
 After=libvirtd.service
-- 
2.7.4

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

Re: [libvirt] [PATCH 0/4] systemd-related fixes and improvements

[Daniel P. Berrange]

On Tue, Sep 06, 2016 at 03:55:20PM +0200, Andrea Bolognani wrote:
> Make libvirt on systemd nicer for the user, by getting rid of
> some confusing behavior, and overall more solid.
> 
> More details in each specific patch.

ACK to all


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

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

Re: [libvirt] [PATCH] Make sure sys/types.h is included after sys/sysmacros.h

[Daniel P. Berrange]

On Tue, Sep 06, 2016 at 03:46:59PM +0200, Michal Privoznik wrote:
> In the latest glibc, major() and minor() functions are marked as
> deprecated (glibc commit dbab6577):
> 
>   CC   util/libvirt_util_la-vircgroup.lo
> util/vircgroup.c: In function 'virCgroupGetBlockDevString':
> util/vircgroup.c:768:5: error: '__major_from_sys_types' is deprecated:
>   In the GNU C Library, `major' is defined by .
>   For historical compatibility, it is currently defined by
>as well, but we plan to remove this soon.
>   To use `major', include  directly.
>   If you did not intend to use a system-defined macro `major',
>   you should #undef it after including .
>   [-Werror=deprecated-declarations]
>  if (virAsprintf(, "%d:%d ", major(sb.st_rdev), minor(sb.st_rdev)) < 
> 0)
>  ^~
> In file included from /usr/include/features.h:397:0,
>  from /usr/include/bits/libc-header-start.h:33,
>  from /usr/include/stdio.h:28,
>  from ../gnulib/lib/stdio.h:43,
>  from util/vircgroup.c:26:
> /usr/include/sys/sysmacros.h:87:1: note: declared here
>  __SYSMACROS_DEFINE_MAJOR (__SYSMACROS_FST_IMPL_TEMPL)
>  ^
> 
> Moreover, in the glibc commit, there's suggestion to keep
> ordering of including of header files as implemented here.
> 
> Signed-off-by: Michal Privoznik 
> ---
> 
> This still won't solve the build issue completely as AC_HEADER_MAJOR still
> reports that major() is defined by sys/types.h instead of sys/sysmacros.h.
> But once they fix it, we are good too. Or we can use the following
> workaround in configure.ac:
> 
>   +old_CFLAGS=$CFLAGS
>   +CFLAGS+=" -Werror "
>AC_HEADER_MAJOR
>   +CFLAGS=$old_CFLAGS

It isn't clear that AC_HEADER_MAJOR is actually doing anything
useful for us. Could we not simply remove the use of AC_HEADER_MAJOR
in configure.ac ? If not, then just re-implement the functionality
ourselves, so that we prioritize checking sys/sysmacros.h in
configure.


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

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

[libvirt] [PATCH 3/4] libvirt-guests.service: Split After= relationship

[Andrea Bolognani]

We use a separate line for each After= relationship in other
unit files: do the same here for consistency's sake, and also
to make future changes nicer to diff
---
 tools/libvirt-guests.service.in | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/libvirt-guests.service.in b/tools/libvirt-guests.service.in
index 66f6cc2..3c901b9 100644
--- a/tools/libvirt-guests.service.in
+++ b/tools/libvirt-guests.service.in
@@ -1,6 +1,8 @@
 [Unit]
 Description=Suspend/Resume Running libvirt Guests
-After=network.target libvirtd.service time-sync.target
+After=network.target
+After=time-sync.target
+After=libvirtd.service
 Documentation=man:libvirtd(8)
 Documentation=http://libvirt.org
 
-- 
2.7.4

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

[libvirt] [PATCH 1/4] virtlogd.socket: Tie lifecycle to libvirtd.service

[Andrea Bolognani]

We already guarantee that virtlogd.socket is enabled/disabled
along with libvirtd.service, but if libvirtd.service has just
been installed and is started before rebooting, then
virtlogd.socket will not be running and guest startup will
fail.

Add Requires=virtlogd.socket to libvirtd.service to make sure
virtlogd.socket is always started along with libvirtd.service,
and add Before=libvirtd.service to both virtlogd.socket and
virtlogd.service so that virtlogd never disappears before
libvirtd has exited.

Also add PartOf=libvirtd.service to both virtlogd.socket and
virtlogd.service, so that virtlogd can be shut down when not
needed.

Resolves: https://bugzilla.redhat.com/1372576
---
 daemon/libvirtd.service.in  | 1 +
 src/logging/virtlogd.service.in | 2 ++
 src/logging/virtlogd.socket.in  | 2 ++
 3 files changed, 5 insertions(+)

diff --git a/daemon/libvirtd.service.in b/daemon/libvirtd.service.in
index 1616e7a..bbf27da 100644
--- a/daemon/libvirtd.service.in
+++ b/daemon/libvirtd.service.in
@@ -5,6 +5,7 @@
 
 [Unit]
 Description=Virtualization daemon
+Requires=virtlogd.socket
 Before=libvirt-guests.service
 After=network.target
 After=dbus.service
diff --git a/src/logging/virtlogd.service.in b/src/logging/virtlogd.service.in
index a264d3a..8287994 100644
--- a/src/logging/virtlogd.service.in
+++ b/src/logging/virtlogd.service.in
@@ -1,6 +1,8 @@
 [Unit]
 Description=Virtual machine log manager
 Requires=virtlogd.socket
+Before=libvirtd.service
+PartOf=libvirtd.service
 Documentation=man:virtlogd(8)
 Documentation=http://libvirt.org
 
diff --git a/src/logging/virtlogd.socket.in b/src/logging/virtlogd.socket.in
index 724976d..efb6504 100644
--- a/src/logging/virtlogd.socket.in
+++ b/src/logging/virtlogd.socket.in
@@ -1,5 +1,7 @@
 [Unit]
 Description=Virtual machine log manager socket
+Before=libvirtd.service
+PartOf=libvirtd.service
 
 [Socket]
 ListenStream=@localstatedir@/run/libvirt/virtlogd-sock
-- 
2.7.4

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

[libvirt] [PATCH 0/4] systemd-related fixes and improvements

[Andrea Bolognani]

Make libvirt on systemd nicer for the user, by getting rid of
some confusing behavior, and overall more solid.

More details in each specific patch.


Andrea Bolognani (4):
  virtlogd.socket: Tie lifecycle to libvirtd.service
  libvirt-guests.service: Improve description
  libvirt-guests.service: Split After= relationship
  libvirt-guests.service: Add Requires=libvirtd.service

 daemon/libvirtd.service.in  | 1 +
 src/logging/virtlogd.service.in | 2 ++
 src/logging/virtlogd.socket.in  | 2 ++
 tools/libvirt-guests.service.in | 7 +--
 4 files changed, 10 insertions(+), 2 deletions(-)

-- 
2.7.4

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

[libvirt] [PATCH 2/4] libvirt-guests.service: Improve description

[Andrea Bolognani]

libvirt-guests.service does both suspend *and* resume guests,
depending on whether it's being started or stopped: the
description should reflect this, to avoid confusing messages
during startup.

Replace "active" with "running" (to match virsh list's output)
and don't capitalize libvirt.
---
 tools/libvirt-guests.service.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libvirt-guests.service.in b/tools/libvirt-guests.service.in
index c31f663..66f6cc2 100644
--- a/tools/libvirt-guests.service.in
+++ b/tools/libvirt-guests.service.in
@@ -1,5 +1,5 @@
 [Unit]
-Description=Suspend Active Libvirt Guests
+Description=Suspend/Resume Running libvirt Guests
 After=network.target libvirtd.service time-sync.target
 Documentation=man:libvirtd(8)
 Documentation=http://libvirt.org
-- 
2.7.4

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

[libvirt] [PATCH] Make sure sys/types.h is included after sys/sysmacros.h

[Michal Privoznik]

In the latest glibc, major() and minor() functions are marked as
deprecated (glibc commit dbab6577):

  CC   util/libvirt_util_la-vircgroup.lo
util/vircgroup.c: In function 'virCgroupGetBlockDevString':
util/vircgroup.c:768:5: error: '__major_from_sys_types' is deprecated:
  In the GNU C Library, `major' is defined by .
  For historical compatibility, it is currently defined by
   as well, but we plan to remove this soon.
  To use `major', include  directly.
  If you did not intend to use a system-defined macro `major',
  you should #undef it after including .
  [-Werror=deprecated-declarations]
 if (virAsprintf(, "%d:%d ", major(sb.st_rdev), minor(sb.st_rdev)) < 0)
 ^~
In file included from /usr/include/features.h:397:0,
 from /usr/include/bits/libc-header-start.h:33,
 from /usr/include/stdio.h:28,
 from ../gnulib/lib/stdio.h:43,
 from util/vircgroup.c:26:
/usr/include/sys/sysmacros.h:87:1: note: declared here
 __SYSMACROS_DEFINE_MAJOR (__SYSMACROS_FST_IMPL_TEMPL)
 ^

Moreover, in the glibc commit, there's suggestion to keep
ordering of including of header files as implemented here.

Signed-off-by: Michal Privoznik 
---

This still won't solve the build issue completely as AC_HEADER_MAJOR still
reports that major() is defined by sys/types.h instead of sys/sysmacros.h.
But once they fix it, we are good too. Or we can use the following
workaround in configure.ac:

  +old_CFLAGS=$CFLAGS
  +CFLAGS+=" -Werror "
   AC_HEADER_MAJOR
  +CFLAGS=$old_CFLAGS

 src/conf/domain_audit.c  | 3 ++-
 src/lxc/lxc_controller.c | 2 +-
 src/lxc/lxc_driver.c | 2 +-
 src/util/vircgroup.c | 2 +-
 src/util/virutil.c   | 2 +-
 5 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
index 53a58ac..52dea02 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -24,7 +24,6 @@
 #include 
 
 #include 
-#include 
 
 #ifdef MAJOR_IN_MKDEV
 # include 
@@ -32,6 +31,8 @@
 # include 
 #endif
 
+#include 
+
 #include "domain_audit.h"
 #include "viraudit.h"
 #include "viruuid.h"
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 825b4d4..8c581df 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -27,7 +27,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #ifdef MAJOR_IN_MKDEV
 # include 
@@ -35,6 +34,7 @@
 # include 
 #endif
 
+#include 
 #include 
 #include 
 #include 
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index da98b38..24025d1 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -28,7 +28,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #ifdef MAJOR_IN_MKDEV
 # include 
@@ -36,6 +35,7 @@
 # include 
 #endif
 
+#include 
 #include 
 #include 
 #include 
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index f2477d5..8b52816 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -34,7 +34,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #ifdef MAJOR_IN_MKDEV
 # include 
@@ -42,6 +41,7 @@
 # include 
 #endif
 
+#include 
 #include 
 #include 
 #include 
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 170dd59..b57a195 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -35,7 +35,6 @@
 #include 
 #include 
 #include 
-#include 
 
 #ifdef MAJOR_IN_MKDEV
 # include 
@@ -43,6 +42,7 @@
 # include 
 #endif
 
+#include 
 #include 
 #include 
 #include 
-- 
2.8.4

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

Re: [libvirt] virt-admin commands aliases

[Erik Skultety]

On 06/09/16 11:52, Martin Kletzander wrote:
> On Tue, Sep 06, 2016 at 09:02:19AM +0200, Erik Skultety wrote:
>> On 05/09/16 19:48, Daniel P. Berrange wrote:
>>> On Mon, Sep 05, 2016 at 05:37:07PM +0200, Erik Skultety wrote:
 Hi there,

 after my presentation at KVM Forum, it was pointed out from the
 audience
 that we might think about doing something about the naming of the
 virt-admin's comands, since there is some sort of inconsistency: srv-
 vs. client- vs. dmn- (not merged yet). When I sent patches to
 upstream I
 already knew that the naming was not optimal, but I didn't come up with
 anything better so I hoped that the reviewer might think of something
 better which unfortunately did not happen.

 Anyway, there are multiple options how this can be approached but I'm
 not 100% satisfied with neither of them:

 1) rename the commands completely
 Although clean, obviously this is the non-preferred option because this
 would break any backwards compatibility however, I think there is a
 fair
 chance that people haven't actually started using it yet (although that
 might change between 7.3 and 7.4).

 2) create aliases for non-abbreviated forms of the commands
 That way, srv- would become server- and dmn- would become daemon-.
 However, by doing this we'll end up with 6 almost identical entries in
 the commands structure which might be error-prone once we decide to
 add/create a flag to the command primitive, since the flag would
 have to be added both to the alias and to the original (unlikely, but
 possible that someone might forget about that)

 3) abbreviate client- to something like clnt-
 Identical to the above except for the amount of duplicate entries which
 would be reduced to 2

 4) leave it as is if such a consensus is reached and accepted
 I guess this does no need any additional comments.
>>>
>>> I just vote for 4.
>>>
>>> In retrospect it would have been nice to use 'server' instead of
>>> 'srv', but ultimately it isn't a functional problem.  The "solutions"
>>> create extra code and/or inconsitency and/or break back-compat so just
>>> aren't worth it IMHO.
>>>
>>
>> Yeah, for me personally, it was either number 2 or 4 but as you write,
>> both of them suck in their own way and I just could not decide which one
>> sucked less.
>>
>> Thanks for opinions guys, appreciated :)
>>
> 
> I, honestly, would go with #2.  I know you know it, but to recapitulate;
> we already have a wiring in the code for aliases, so if you change:
> 
>{.name = "srv-list",
> .handler = cmdSrvList,
> .opts = NULL,
> .info = info_srv_list,
> .flags = 0
>},
> 
> to:
>{.name = "server-list",
> .handler = cmdSrvList,
> .opts = NULL,
> .info = info_srv_list,
> .flags = 0
>},
>{.name = "srv-list",
> .handler = cmdSrvList,
> .opts = NULL,
> .info = info_srv_list,
> .flags = VSH_CMD_FLAG_ALIAS
>},
> 
> You will have both commands, you will only see the 'server-list' in the
> help and both will work.  The only thing you need to change is, that if
> you add options to the commands that don't have any (yet), you need to
> add the opts_srv_list to both commands.  But that's it.  For
> srv-clients-list (BTW why isn't it named list-clients or client-list)
> that would require no future change.  I know you mentioned that flag
> changes would need to be done in both structures as well, but honestly,
> thre's VSH_CMD_FLAG_NOCONNECT and VSH_CMD_FLAG_ALIAS.  And that's it.
> And I don't think we'll have any new flags anytime soon.  I even dare to
> say never.
> 
> But even if you don't like that, there are ways to mitigate even this
> duplication.  Just make VSH_CMD_FLAG_ALIAS work as VSH_OT_ALIAS.  That
> is just make the following work:
> 
>{.name = "server-list",
> .handler = cmdSrvList,
> .opts = NULL,
> .info = info_srv_list,
> .flags = 0
>},
>{.name = "srv-list",
> .flags = VSH_CMD_FLAG_ALIAS,
> .aliased = "server-list"
>},
> 

Hmm, well you'll still end up with N more entries (possibly) creating
more unnecessary "noise" but I have to say that I like ^this proposal.
I'll try to play with it, send the patches and we'll see. If the patches
will end up being NACKed, fair enough, time wasted but I think I can
live with that.

> When parsing you'll see that it's an alias and return the opts, flags
> and info for the original aliased command.  While on that, you can make
> it even better; you can get rid of flags completely by just checking if
> 'aliased' is set.  If it is, just search for the original command and
> that's it.
> 
> I'd vote for doing something like this and maybe keeping the aliases for
> daemon and server added for new commands as well (that is when you add
> server-make-me-a-sandwich, you also add srv- alias as well), but that's
> not a hard requirement.

aaand I'm 

Re: [libvirt] [PATCH] virsh: Fix *-event error string

[Erik Skultety]

On 06/09/16 13:17, Christophe Fergeau wrote:
> When using
> virsh net-event non-existing-net
> the error message says that 'either --list or event type is required'
> This is misleading as 'virsh net-event $valid-event-type' is not going
> to work either. What is expected is 'virsh net-event --event
> $valid-event-type'
> 
> This commit fixes the string in pool-event, nodedev-event, event, and
> net-event.
> ---
>  tools/virsh-domain.c  | 2 +-
>  tools/virsh-network.c | 2 +-
>  tools/virsh-nodedev.c | 2 +-
>  tools/virsh-pool.c| 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index a614512..702a8bd8 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -12694,7 +12694,7 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
>  }
>  } else if (!all) {
>  vshError(ctl, "%s",
> - _("one of --list, --all, or event type is required"));
> + _("one of --list, --all, or --event type is required"));
>  return false;
>  }
>  
> diff --git a/tools/virsh-network.c b/tools/virsh-network.c
> index eec7faf..c6bd132 100644
> --- a/tools/virsh-network.c
> +++ b/tools/virsh-network.c
> @@ -1238,7 +1238,7 @@ cmdNetworkEvent(vshControl *ctl, const vshCmd *cmd)
>  if (vshCommandOptStringReq(ctl, cmd, "event", ) < 0)
>  return false;
>  if (!eventName) {
> -vshError(ctl, "%s", _("either --list or event type is required"));
> +vshError(ctl, "%s", _("either --list or --event type is required"));
>  return false;
>  }
>  if ((event = virshNetworkEventIdTypeFromString(eventName)) < 0) {
> diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
> index 805c0ff..0e695b9 100644
> --- a/tools/virsh-nodedev.c
> +++ b/tools/virsh-nodedev.c
> @@ -903,7 +903,7 @@ cmdNodeDeviceEvent(vshControl *ctl, const vshCmd *cmd)
>  if (vshCommandOptStringReq(ctl, cmd, "event", ) < 0)
>  return false;
>  if (!eventName) {
> -vshError(ctl, "%s", _("either --list or event type is required"));
> +vshError(ctl, "%s", _("either --list or --event type is required"));
>  return false;
>  }
>  
> diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
> index d25851e..70b2bdd 100644
> --- a/tools/virsh-pool.c
> +++ b/tools/virsh-pool.c
> @@ -2057,7 +2057,7 @@ cmdPoolEvent(vshControl *ctl, const vshCmd *cmd)
>  if (vshCommandOptStringReq(ctl, cmd, "event", ) < 0)
>  return false;
>  if (!eventName) {
> -vshError(ctl, "%s", _("either --list or event type is required"));
> +vshError(ctl, "%s", _("either --list or --event type is required"));
>  return false;
>  }
>  

ACK, although, I have a really small nitpick the decision on fixing of
which I leave to you. After the fix, "--list or --event type...", at
least to me, sounds like "--event" was the mentioned type itself which
is of course wrong. Maybe it could be better (just an idea) to use angle
brackets like this '' to make the message more clear, but as I
said, your call.

Regards,
Erik

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

[libvirt] [PATCH v2 4/5] 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/conf/domain_conf.c  |   6 ++
 src/qemu/qemu_hotplug.c | 148 
 2 files changed, 154 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 45b643b..123bbcb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13720,6 +13720,12 @@ virDomainHostdevMatchSubsys(virDomainHostdevDefPtr a,
 return virDomainHostdevMatchSubsysSCSIiSCSI(a, b);
 else
 return virDomainHostdevMatchSubsysSCSIHost(a, b);
+case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST:
+if (a->source.subsys.u.host.protocol !=
+b->source.subsys.u.host.protocol)
+return 0;
+if (STREQ(a->source.subsys.u.host.wwpn, b->source.subsys.u.host.wwpn))
+return 1;
 }
 return 0;
 }
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index e9140a9..5f8d411 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2165,6 +2165,119 @@ 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;
+size_t vhostfdSize = 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 (qemuSetupHostdevCgroup(vm, hostdev) < 0)
+goto cleanup;
+teardowncgroup = true;
+
+if (virSecurityManagerSetHostdevLabel(driver->securityManager,
+  vm->def, hostdev, NULL) < 0)
+goto cleanup;
+teardownlabel = true;
+
+if (virSCSIOpenVhost(, ) < 0)
+goto cleanup;
+
+if (virAsprintf(, "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, >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, >info->alias, -1) < 0)
+goto cleanup;
+
+qemuDomainObjEnterMonitor(driver, vm);
+
+if (hostdev->source.subsys.u.host.protocol == 
VIR_DOMAIN_HOSTDEV_HOST_PROTOCOL_TYPE_VHOST) {
+if (!(devstr = qemuBuildHostHostdevDevStr(vm->def,
+  hostdev,
+  priv->qemuCaps,
+  vhostfdName,
+  vhostfdSize)))
+goto exit_monitor;
+
+if ((ret = qemuMonitorAddDeviceWithFd(priv->mon,
+  devstr,
+  vhostfd,
+  vhostfdName)) < 0) {
+goto exit_monitor;
+}
+}
+
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+goto cleanup;
+
+virDomainAuditHostdev(vm, hostdev, "attach", true);
+

[libvirt] [PATCH RESEND v2 0/5] Implementation of QEMU vhost-scsi

[Eric Farman]

[Resending after the release of 2.2.0; no changes other than a rebase
to current master and the associated tweaking to the capabilities patch]

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].

There is no capability check in this series for vhost-scsi in the underlying
QEMU.  Using a recent QEMU built with --disable-vhost-scsi fails with "not a
valid device model name."

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->v2.1:
   - Rebased to current master (6 September)

  v1->v2:  https://www.redhat.com/archives/libvir-list/2016-August/msg01028.html
   - Rebase
 - Applies to current master (20 August)
 - Added a capability check for QEMU 2.7
 - Fixed the qemuxml2argv tests as the -smp options had changed
 - Reworked ccwaddrs parameter in virDomainCCWAddressAssign call
   - Comments
 - Squashed documentation, XML schema, XML parsing, and infrastructure
   patches into a single patch
 - Switched from "hostdev type='scsi'" to "hostdev type='scsi_host'";
   this removes the refactoring patches since we're not shoe-horning
   a new SCSI protocol into the existing code
 - Reworked the handling of the fd's such that we send them to qemu
   after any possible errors could occur and cause us to back out
 - s/qemuBuildSCSIVhostHostdevDevStr/qemuBuildHostHostdevDevStr/
 - Added virBufferCheckError, and an error message for vhostfdSize > 1,
   in qemuBuildHostHostdevDevStr
 - Added qemuBuildDeviceAddressStr in qemuBuildHostHostdevDevStr, thus
   superceding the last patch in the v1 series
   - Other
 - Simplified the vhostfd logic to just be a single int, rather than
   an alloc'd array (left the vhostfdSize described above as an
   identifier for if QEMU ever supports multiple vhostfds)
 - Replaced "qemuMonitorAddDevice" with "qemuMonitorAddDeviceWithFd"
   in hotplug routine

  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 (5):
  Introduce a "scsi_host" hostdev type
  qemu: Introduce vhost-scsi capability
  qemu: Add vhost-scsi string for -device parameter
  qemu: Allow hotplug of vhost-scsi device
  tests: Introduce basic vhost-scsi test

 docs/formatdomain.html.in  |  24 
 docs/schemas/domaincommon.rng  |  23 
 src/conf/domain_audit.c|   2 +
 src/conf/domain_conf.c |  62 -
 src/conf/domain_conf.h |  17 +++
 src/libvirt_private.syms   |   1 +
 src/qemu/qemu_capabilities.c   |   3 +
 src/qemu/qemu_capabilities.h   |   3 +
 src/qemu/qemu_command.c|  80 +++
 src/qemu/qemu_command.h|   6 +
 src/qemu/qemu_domain_address.c |  10 ++
 src/qemu/qemu_hotplug.c| 149 +
 src/security/security_dac.c|   2 +
 src/util/virscsi.c |  26 
 src/util/virscsi.h |   1 +
 tests/domaincapsschemadata/full.xml|   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 +
 .../caps_2.6.0-gicv2.aarch64.xml   |   1 +
 .../caps_2.6.0-gicv3.aarch64.xml   |  

[libvirt] [PATCH v2 1/5] 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, and its associated documentation and XML schema information.

Signed-off-by: Eric Farman 
---
 docs/formatdomain.html.in   | 24 
 docs/schemas/domaincommon.rng   | 23 +++
 src/conf/domain_audit.c |  2 ++
 src/conf/domain_conf.c  | 56 +++--
 src/conf/domain_conf.h  | 17 +++
 src/qemu/qemu_domain_address.c  | 10 +++
 src/qemu/qemu_hotplug.c |  1 +
 src/security/security_dac.c |  2 ++
 tests/domaincapsschemadata/full.xml |  1 +
 9 files changed, 134 insertions(+), 2 deletions(-)

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index feaeaa2..b79da95 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3655,6 +3655,17 @@
   /devices
   ...
 
+or:
+
+
+  ...
+  devices
+hostdev mode='subsystem' type='scsi_host'
+  source protocol='vhost' wwpn='naa.50014057667280d8'/
+/hostdev
+  /devices
+  ...
+
 
   hostdev
   The hostdev element is the main container for describing
@@ -3693,6 +3704,12 @@
 If a disk lun in the domain already has the rawio capability,
 then this setting not required.
   
+  scsi_host
+  since 2.2.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
@@ -3756,6 +3773,13 @@
 credentials to the iSCSI server.
 
   
+  scsi_host
+  Since 2.2.0, multiple LUNs behind a
+single SCSI HBI 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
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index af32df8..7fd6bd2 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3938,6 +3938,7 @@
   
   
   
+  
 
   
 
@@ -4066,6 +4067,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 53a58ac..406dd8f 100644
--- a/src/conf/domain_audit.c
+++ b/src/conf/domain_audit.c
@@ -443,6 +443,8 @@ virDomainAuditHostdev(virDomainObjPtr vm, 
virDomainHostdevDefPtr hostdev,
 }
 break;
 }
+case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST:
+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 c8c4f61..45b643b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -646,7 +646,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,
@@ -660,6 +661,10 @@ VIR_ENUM_IMPL(virDomainHostdevSubsysSCSIProtocol,
   "adapter",
   "iscsi")
 
+VIR_ENUM_IMPL(virDomainHostdevSubsysHostProtocol,
+  VIR_DOMAIN_HOSTDEV_HOST_PROTOCOL_TYPE_LAST,
+  "vhost")
+
 VIR_ENUM_IMPL(virDomainHostdevCaps, VIR_DOMAIN_HOSTDEV_CAPS_TYPE_LAST,
   "storage",
   "misc",
@@ -2270,6 +2275,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 = >source.subsys.u.host;
+VIR_FREE(hostsrc->wwpn);
 }
 break;
 }
@@ -5977,6 +5985,31 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr 
sourcenode,
 return ret;
 }
 
+static int
+virDomainHostdevSubsysHostDefParseXML(xmlNodePtr sourcenode,
+  virDomainHostdevDefPtr def)
+{
+

[libvirt] [PATCH v2 5/5] tests: Introduce basic vhost-scsi test

[Eric Farman]

The qemuxml2argv test was cloned from hostdev-scsi-virtio-scsi

Signed-off-by: Eric Farman 
Reviewed-by: Boris Fiuczynski 
---
 .../qemuxml2argv-hostdev-scsi-vhost-scsi.args  | 24 
 .../qemuxml2argv-hostdev-scsi-vhost-scsi.xml   | 33 ++
 tests/qemuxml2argvmock.c   | 12 
 tests/qemuxml2argvtest.c   |  3 ++
 4 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

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..5b7cc02
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-hostdev-scsi-vhost-scsi.xml
@@ -0,0 +1,33 @@
+
+  QEMUGuest2
+  c7a5fdbd-edaf-9466-926a-d65c16db1809
+  219100
+  219100
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu
+
+  
+  
+  
+
+
+
+
+
+
+
+
+  
+
+
+  
+
diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c
index 78a224b..568dcc0 100644
--- a/tests/qemuxml2argvmock.c
+++ b/tests/qemuxml2argvmock.c
@@ -107,6 +107,18 @@ virSCSIDeviceGetSgName(const char *sysfs_prefix 
ATTRIBUTE_UNUSED,
 }
 
 int
+virSCSIOpenVhost(int *vhostfd,
+ size_t *vhostfdSize)
+{
+size_t i;
+
+for (i = 0; i < *vhostfdSize; i++)
+vhostfd[i] = STDERR_FILENO + 1 + i;
+
+return 0;
+}
+
+int
 virNetDevTapCreate(char **ifname,
const char *tunpath ATTRIBUTE_UNUSED,
int *tapfd,
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 39abe72..99c0465 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1783,6 +1783,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);
-- 
1.9.1

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

[libvirt] [PATCH v2 3/5] 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/libvirt_private.syms |  1 +
 src/qemu/qemu_command.c  | 80 
 src/qemu/qemu_command.h  |  6 
 src/util/virscsi.c   | 26 
 src/util/virscsi.h   |  1 +
 5 files changed, 114 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d62c74c..22fe14d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2265,6 +2265,7 @@ virSCSIDeviceListNew;
 virSCSIDeviceListSteal;
 virSCSIDeviceNew;
 virSCSIDeviceSetUsedBy;
+virSCSIOpenVhost;
 
 
 # util/virseclabel.h
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 982c33c..479dde2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4711,6 +4711,52 @@ qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr 
dev)
 }
 
 char *
+qemuBuildHostHostdevDevStr(const virDomainDef *def,
+   virDomainHostdevDefPtr dev,
+   virQEMUCapsPtr qemuCaps,
+   char *vhostfdName,
+   size_t vhostfdSize)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+virDomainHostdevSubsysHostPtr hostsrc = >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(, "vhost-scsi-ccw");
+else
+virBufferAddLit(, "vhost-scsi-pci");
+
+virBufferAsprintf(, ",wwpn=%s", hostsrc->wwpn);
+
+if (vhostfdSize == 1) {
+virBufferAsprintf(, ",vhostfd=%s", vhostfdName);
+} else {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("QEMU supports a single vhost-scsi file descriptor"));
+goto cleanup;
+}
+
+virBufferAsprintf(, ",id=%s", dev->info->alias);
+
+if (qemuBuildDeviceAddressStr(, def, dev->info, qemuCaps) < 0)
+goto cleanup;
+
+if (virBufferCheckError() < 0)
+goto cleanup;
+
+return virBufferContentAndReset();
+
+ cleanup:
+virBufferFreeAndReset();
+return NULL;
+}
+
+char *
 qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev)
 {
 virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -5156,6 +5202,40 @@ 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_DEVICE_SCSI_GENERIC)) {
+if (hostdev->source.subsys.u.host.protocol == 
VIR_DOMAIN_HOSTDEV_HOST_PROTOCOL_TYPE_VHOST) {
+char *vhostfdName = NULL;
+int vhostfd = -1;
+size_t vhostfdSize = 1;
+
+if (virSCSIOpenVhost(, ) < 0)
+return -1;
+
+if (virAsprintf(, "%d", vhostfd) < 0) {
+VIR_FORCE_CLOSE(vhostfd);
+return -1;
+}
+
+virCommandPassFD(cmd, vhostfd, 
VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+
+virCommandAddArg(cmd, "-device");
+if (!(devstr = qemuBuildHostHostdevDevStr(def, hostdev, 
qemuCaps, vhostfdName, vhostfdSize)))
+return -1;
+virCommandAddArg(cmd, devstr);
+
+VIR_FREE(vhostfdName);
+VIR_FREE(devstr);
+}
+} else {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+   _("SCSI passthrough is not supported by this 
version of qemu"));
+return -1;
+}
+}
 }
 
 return 0;
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 9b9ccb6..78179de 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -158,6 +158,12 @@ char *qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr 
dev);
 char *qemuBuildSCSIHostdevDevStr(const virDomainDef *def,
  virDomainHostdevDefPtr dev,
  virQEMUCapsPtr qemuCaps);
+char *
+qemuBuildHostHostdevDevStr(const virDomainDef *def,
+   virDomainHostdevDefPtr dev,
+   virQEMUCapsPtr qemuCaps,
+   char 

[libvirt] [PATCH v2 2/5] 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| 3 +++
 src/qemu/qemu_capabilities.h| 3 +++
 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, 17 insertions(+)

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 2ca7803..cf1e468 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -342,6 +342,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
   "smm",
   "virtio-pci-disable-legacy",
   "query-hotpluggable-cpus",
+
+  "vhost-scsi", /* 235 */
 );
 
 
@@ -1566,6 +1568,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 a74d39f..d8ddc8f 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -376,6 +376,9 @@ typedef enum {
 QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY, /* virtio-*pci.disable-legacy */
 QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS, /* qmp command query-hotpluggable-cpus 
*/
 
+/* 235 */
+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 0d048da..eeaf3d5 100644
--- a/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_1.5.3.x86_64.xml
@@ -143,6 +143,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 a6d4561..b72c4df 100644
--- a/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_1.6.0.x86_64.xml
@@ -148,6 +148,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 f756a41..74a4292 100644
--- a/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_1.7.0.x86_64.xml
@@ -150,6 +150,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 a77ad9e..cc829da 100644
--- a/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.1.1.x86_64.xml
@@ -165,6 +165,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 db778ef..020ea44 100644
--- a/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.4.0.x86_64.xml
@@ -185,6 +185,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 fc915ad..730d755 100644
--- a/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_2.5.0.x86_64.xml
@@ -190,6 +190,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 fd14665..73eb588 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 eb708f8..6143af3 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 

Re: [libvirt] [PATCH] virsh: use virConnectGetDomainCapabilities with maxvcpus

[Peter Krempa]

On Tue, Sep 06, 2016 at 16:59:09 +0530, Shivaprasad G Bhat wrote:
> virsh maxvcpus --type kvm output is useless on PPC. Also, in
> commit e6806d79 we documented not rely on virConnectGetMaxVcpus
> output. Fix the  maxvcpus to use virConnectGetDomainCapabilities
> now to make it useful. The call is made to use the default emulator
> binary and to check for the host machine and arch which is what the
> command intends to show anyway.
> 
> Signed-off-by: Shivaprasad G Bhat 
> ---
>  tools/virsh-host.c |   28 +---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/virsh-host.c b/tools/virsh-host.c
> index 57f0c0e..505cfbb 100644
> --- a/tools/virsh-host.c
> +++ b/tools/virsh-host.c
> @@ -607,16 +607,38 @@ cmdMaxvcpus(vshControl *ctl, const vshCmd *cmd)
>  {
>  const char *type = NULL;
>  int vcpus;
> +char *caps = NULL;
> +const unsigned int flags = 0; /* No flags so far */
> +xmlDocPtr xml = NULL;
> +xmlXPathContextPtr ctxt = NULL;
>  virshControlPtr priv = ctl->privData;
>  
>  if (vshCommandOptStringReq(ctl, cmd, "type", ) < 0)
>  return false;
>  
> -if ((vcpus = virConnectGetMaxVcpus(priv->conn, type)) < 0)
> -return false;
> +caps = virConnectGetDomainCapabilities(priv->conn, NULL, NULL, NULL, 
> type, flags);
> +if (caps) {
> +xml = virXMLParseStringCtxt(caps, _("(domainCapabilities)"), );
> +if (!xml) {
> +VIR_FREE(caps);
> +return false;
> +}
>  
> -vshPrint(ctl, "%d\n", vcpus);
> +virXPathInt("string(./vcpu[1]/@max)", ctxt, );

This doesn't handle the case when the capability XML does not contain
the required data. This still should fall back to the legacy approach.

Additionally virXPathInt does not initialize vcpus on failure and since
it's not initialized when declared this would trigger a compiler
warning.

> +xmlXPathFreeContext(ctxt);
> +xmlFreeDoc(xml);
> +VIR_FREE(caps);

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

Re: [libvirt] [PATCH] qemu: Don't warn about missing device in DEVICE_DELETED event

[Peter Krempa]

On Tue, Sep 06, 2016 at 13:47:52 +0200, Jiri Denemark wrote:
> Debug priority is good enough for this.
> 
> Signed-off-by: Jiri Denemark 
> ---
>  src/qemu/qemu_monitor_json.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

ACK

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

[libvirt] [PATCH] tools: Pass opaque data in vshCompleter and introduce autoCompleteOpaque

[Nishith Shah]

This patch changes the signature of vshCompleters, allowing to pass along
some data that we might want to along with the completers; for example,
we might want to pass the autocomplete vshControl along with the
completer, in case the completer requires a connection to libvirtd.

Signed-off-by: Nishith Shah 
---
 tools/vsh.c | 10 +-
 tools/vsh.h |  2 +-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/tools/vsh.c b/tools/vsh.c
index 3157922..3cc03ec 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -64,6 +64,11 @@
 # define SA_SIGINFO 0
 #endif
 
+#ifdef WITH_READLINE
+/* For autocompletion */
+void *autoCompleteOpaque;
+#endif
+
 /* NOTE: It would be much nicer to have these two as part of vshControl
  * structure, unfortunately readline doesn't support passing opaque data
  * and only relies on static data accessible from the user-side callback
@@ -2808,7 +2813,8 @@ vshReadlineParse(const char *text, int state)
 res = vshReadlineOptionsGenerator(sanitized_text, state, cmd);
 } else if (non_bool_opt_exists && data_complete && opt->completer) {
 if (!completed_list)
-completed_list = opt->completer(opt->completer_flags);
+completed_list = opt->completer(autoCompleteOpaque,
+opt->completer_flags);
 if (completed_list) {
 while ((completed_name = completed_list[completed_list_index])) {
 completed_list_index++;
@@ -2858,6 +2864,8 @@ vshReadlineInit(vshControl *ctl)
 char *histsize_env = NULL;
 const char *histsize_str = NULL;
 
+/* Opaque data for autocomplete callbacks and connections to libvirtd */
+autoCompleteOpaque = ctl;
 /* Allow conditional parsing of the ~/.inputrc file.
  * Work around ancient readline 4.1 (hello Mac OS X),
  * which declared it as 'char *' instead of 'const char *'.
diff --git a/tools/vsh.h b/tools/vsh.h
index 7f43055..e53910f 100644
--- a/tools/vsh.h
+++ b/tools/vsh.h
@@ -123,7 +123,7 @@ typedef struct _vshCmdOpt vshCmdOpt;
 typedef struct _vshCmdOptDef vshCmdOptDef;
 typedef struct _vshControl vshControl;
 
-typedef char **(*vshCompleter)(unsigned int flags);
+typedef char **(*vshCompleter)(void *opaque, unsigned int flags);
 
 /*
  * vshCmdInfo -- name/value pair for information about command
-- 
2.1.4

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

Re: [libvirt] [PATCH 0/2] util: storage: Fixes for the JSON pseudo protocol parser

[Jiri Denemark]

On Mon, Sep 05, 2016 at 18:42:45 +0200, Peter Krempa wrote:
> Gluster protocol type was not set properly and the RBD protocol was missing.
> 
> Peter Krempa (2):
>   util: storage: Properly set protocol type when parsing gluster json
> string
>   util: storage: Add json pseudo protocol support for legacy RBD strings
> 
>  src/util/virstoragefile.c | 26 ++
>  tests/virstoragetest.c| 10 --

ACK both

Jirka

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

[libvirt] [PATCH] qemu: Don't warn about missing device in DEVICE_DELETED event

[Jiri Denemark]

Debug priority is good enough for this.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_monitor_json.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 3d0a120..8384314 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -941,7 +941,7 @@ qemuMonitorJSONHandleDeviceDeleted(qemuMonitorPtr mon, 
virJSONValuePtr data)
 const char *device;
 
 if (!(device = virJSONValueObjectGetString(data, "device"))) {
-VIR_WARN("missing device in device deleted event");
+VIR_DEBUG("missing device in device deleted event");
 return;
 }
 
-- 
2.10.0

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

[libvirt] [PATCH] virsh: use virConnectGetDomainCapabilities with maxvcpus

[Shivaprasad G Bhat]

virsh maxvcpus --type kvm output is useless on PPC. Also, in
commit e6806d79 we documented not rely on virConnectGetMaxVcpus
output. Fix the  maxvcpus to use virConnectGetDomainCapabilities
now to make it useful. The call is made to use the default emulator
binary and to check for the host machine and arch which is what the
command intends to show anyway.

Signed-off-by: Shivaprasad G Bhat 
---
 tools/virsh-host.c |   28 +---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index 57f0c0e..505cfbb 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -607,16 +607,38 @@ cmdMaxvcpus(vshControl *ctl, const vshCmd *cmd)
 {
 const char *type = NULL;
 int vcpus;
+char *caps = NULL;
+const unsigned int flags = 0; /* No flags so far */
+xmlDocPtr xml = NULL;
+xmlXPathContextPtr ctxt = NULL;
 virshControlPtr priv = ctl->privData;
 
 if (vshCommandOptStringReq(ctl, cmd, "type", ) < 0)
 return false;
 
-if ((vcpus = virConnectGetMaxVcpus(priv->conn, type)) < 0)
-return false;
+caps = virConnectGetDomainCapabilities(priv->conn, NULL, NULL, NULL, type, 
flags);
+if (caps) {
+xml = virXMLParseStringCtxt(caps, _("(domainCapabilities)"), );
+if (!xml) {
+VIR_FREE(caps);
+return false;
+}
 
-vshPrint(ctl, "%d\n", vcpus);
+virXPathInt("string(./vcpu[1]/@max)", ctxt, );
+xmlXPathFreeContext(ctxt);
+xmlFreeDoc(xml);
+VIR_FREE(caps);
+} else {
+if (last_error && last_error->code != VIR_ERR_NO_SUPPORT)
+return false;
+
+vshResetLibvirtError();
 
+if ((vcpus = virConnectGetMaxVcpus(priv->conn, type)) < 0)
+return false;
+}
+
+vshPrint(ctl, "%d\n", vcpus);
 return true;
 }
 

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

Re: [libvirt] [PATCH] docs: Add libvirt-go Go bindings to binding page

[Michal Privoznik]

On 06.09.2016 12:47, Roman Mohr wrote:
> Signed-off-by: Roman Mohr 
> ---
>  docs/bindings.html.in | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/docs/bindings.html.in b/docs/bindings.html.in
> index 95cfe25..7fe26df 100644
> --- a/docs/bindings.html.in
> +++ b/docs/bindings.html.in
> @@ -14,6 +14,10 @@
>  C#: Arnaud Champion develops
>  C# bindings.
>
> + 
> +Go: Kyle Kelley et al. are developing
> +https://github.com/rgbkrk/libvirt-go;>Go bindings.
> +  
>
>  Java: Daniel Veillard develops
>  Java bindings.
> 

ACKed and pushed. Congratulations on your first libvirt contribution!

Michal

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

Re: [libvirt] [PATCH 1/2] qemu: add a max_core setting to qemu.conf for core dump size

[Daniel P. Berrange]

On Wed, Aug 17, 2016 at 05:04:48PM -0400, John Ferlan wrote:
> 
> 
> On 08/03/2016 11:31 AM, Daniel P. Berrange wrote:
> > Currently the QEMU processes inherit their core dump rlimit
> > from libvirtd, which is really suboptimal. This change allows
> > their limit to be directly controlled from qemu.conf instead.
> > ---
> >  src/libvirt_private.syms   |  2 ++
> >  src/qemu/libvirtd_qemu.aug |  4 
> >  src/qemu/qemu.conf | 23 ++-
> >  src/qemu/qemu_conf.c   | 17 +
> >  src/qemu/qemu_conf.h   |  1 +
> >  src/qemu/qemu_process.c|  1 +
> >  src/qemu/test_libvirtd_qemu.aug.in |  1 +
> >  src/util/vircommand.c  | 14 ++
> >  src/util/vircommand.h  |  1 +
> >  src/util/virprocess.c  | 36 
> > 
> >  src/util/virprocess.h  |  1 +
> >  11 files changed, 100 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> > index 419c33d..6dc2b23 100644
> > --- a/src/libvirt_private.syms
> > +++ b/src/libvirt_private.syms
> > @@ -1389,6 +1389,7 @@ virCommandSetErrorFD;
> >  virCommandSetGID;
> >  virCommandSetInputBuffer;
> >  virCommandSetInputFD;
> > +virCommandSetMaxCoreSize;
> >  virCommandSetMaxFiles;
> >  virCommandSetMaxMemLock;
> >  virCommandSetMaxProcesses;
> > @@ -2199,6 +2200,7 @@ virProcessRunInMountNamespace;
> >  virProcessSchedPolicyTypeFromString;
> >  virProcessSchedPolicyTypeToString;
> >  virProcessSetAffinity;
> > +virProcessSetMaxCoreSize;
> >  virProcessSetMaxFiles;
> >  virProcessSetMaxMemLock;
> >  virProcessSetMaxProcesses;
> > diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
> > index 8bc23ba..9ec8250 100644
> > --- a/src/qemu/libvirtd_qemu.aug
> > +++ b/src/qemu/libvirtd_qemu.aug
> > @@ -22,6 +22,9 @@ module Libvirtd_qemu =
> > let int_entry   (kw:string) = [ key kw . value_sep . int_val ]
> > let str_array_entry (kw:string) = [ key kw . value_sep . str_array_val ]
> >  
> > +   let unlimited_val =  del /\"/ "\"" . store /unlimited/ . del /\"/ "\""
> > +   let limits_entry (kw:string) = [ key kw . value_sep . unlimited_val ] | 
> >  [ key kw . value_sep . int_val ]
> > +
> 
> Is there no 'uulong_val'?  At the very least uint?  I don't know much
> about this aug stuff, so I defer to you on this.

There's no built-in types at all in augeas - we just define the grammar
entirely ourselves.

> > (* Config entry grouped by function - same order as example config *)
> > let vnc_entry = str_entry "vnc_listen"
> > @@ -72,6 +75,7 @@ module Libvirtd_qemu =
> >   | bool_entry "set_process_name"
> >   | int_entry "max_processes"
> >   | int_entry "max_files"
> > + | limits_entry "max_core"
> >   | str_entry "stdio_handler"
> >  
> > let device_entry = bool_entry "mac_filter"
> > diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
> > index 7964273..abf9938 100644
> > --- a/src/qemu/qemu.conf
> > +++ b/src/qemu/qemu.conf
> > @@ -401,7 +401,28 @@
> >  #max_processes = 0
> >  #max_files = 0
> >  
> > -
> > +# If max_core is set to a positive integer, then QEMU will be
> > +# permitted to create core dumps when it crashes, provided its
> > +# RAM size is smaller than the limit set.
> 
> Providing a negative value will cause libvirtd startup failure... The
> way this is worded could lead one to believe they could provide a signed
> value.

Ok, i'll reword it.

> > +# Be warned that the core dump will include a full copy of the
> > +# guest RAM, unless it has been disabled via the guest XML by
> > +# setting:
> > +#
> > +#   ...guest ram...
> > +#
> > +# If guest RAM is to be included, ensure the max_core limit
> > +# is set to at least the size of the largest expected guest
> > +# plus another 1GB for any QEMU host side memory mappings.
> > +#
> > +# As a special case it can be set to the string "unlimited" to
> > +# to allow arbitrarily sized core dumps.
> > +#
> > +# By default the core dump size is set to 0 disabling all dumps
> > +#
> > +# Size is in bytes or string "unlimited"
> > +#
> > +#max_core = "unlimited"
> >  
> 
> Would it be overly complicated to alter max_core to be "unlimited" or a
> sized value?  I would think it would be easier to type/see "5G" rather
> than do the math! or fat-finger or cut-n-paste wrong...

Currently our config file format is essentially python variable
initialization syntax, so you can have unquoted numbers, or quoted
strings. So we could not have a bare 5G - it would have to be input
as a string which I think would be confusing - eg users would expect
these to work:

  max_core = 5368709120
  max_core = 5G

but would in fact have to use

  max_core = 5368709120
  max_core = "5G"


we could solve this by extending the virConf parsing code to deal
with bare sized values, but I don't fancy attacking that 

[libvirt] [PATCH] virsh: Fix *-event error string

[Christophe Fergeau]

When using
virsh net-event non-existing-net
the error message says that 'either --list or event type is required'
This is misleading as 'virsh net-event $valid-event-type' is not going
to work either. What is expected is 'virsh net-event --event
$valid-event-type'

This commit fixes the string in pool-event, nodedev-event, event, and
net-event.
---
 tools/virsh-domain.c  | 2 +-
 tools/virsh-network.c | 2 +-
 tools/virsh-nodedev.c | 2 +-
 tools/virsh-pool.c| 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index a614512..702a8bd8 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -12694,7 +12694,7 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd)
 }
 } else if (!all) {
 vshError(ctl, "%s",
- _("one of --list, --all, or event type is required"));
+ _("one of --list, --all, or --event type is required"));
 return false;
 }
 
diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index eec7faf..c6bd132 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -1238,7 +1238,7 @@ cmdNetworkEvent(vshControl *ctl, const vshCmd *cmd)
 if (vshCommandOptStringReq(ctl, cmd, "event", ) < 0)
 return false;
 if (!eventName) {
-vshError(ctl, "%s", _("either --list or event type is required"));
+vshError(ctl, "%s", _("either --list or --event type is required"));
 return false;
 }
 if ((event = virshNetworkEventIdTypeFromString(eventName)) < 0) {
diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c
index 805c0ff..0e695b9 100644
--- a/tools/virsh-nodedev.c
+++ b/tools/virsh-nodedev.c
@@ -903,7 +903,7 @@ cmdNodeDeviceEvent(vshControl *ctl, const vshCmd *cmd)
 if (vshCommandOptStringReq(ctl, cmd, "event", ) < 0)
 return false;
 if (!eventName) {
-vshError(ctl, "%s", _("either --list or event type is required"));
+vshError(ctl, "%s", _("either --list or --event type is required"));
 return false;
 }
 
diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c
index d25851e..70b2bdd 100644
--- a/tools/virsh-pool.c
+++ b/tools/virsh-pool.c
@@ -2057,7 +2057,7 @@ cmdPoolEvent(vshControl *ctl, const vshCmd *cmd)
 if (vshCommandOptStringReq(ctl, cmd, "event", ) < 0)
 return false;
 if (!eventName) {
-vshError(ctl, "%s", _("either --list or event type is required"));
+vshError(ctl, "%s", _("either --list or --event type is required"));
 return false;
 }
 
-- 
2.7.4

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

Re: [libvirt] [PATCH] Prefix major/minor with gnu_dev_

[Andrea Bolognani]

On Mon, 2016-09-05 at 14:47 +0100, Daniel P. Berrange wrote:
> On Mon, Sep 05, 2016 at 03:33:36PM +0200, Michal Privoznik wrote:
> > 
> > In the latest glibc, major() and minor() functions are marked as
> > deprecated (glibc commit dbab6577):
> > 
> >   CC   util/libvirt_util_la-vircgroup.lo
> > util/vircgroup.c: In function 'virCgroupGetBlockDevString':
> > util/vircgroup.c:768:5: error: '__major_from_sys_types' is deprecated:
> >   In the GNU C Library, `major' is defined by .
> >   For historical compatibility, it is currently defined by
> >    as well, but we plan to remove this soon.
> >   To use `major', include  directly.
> >   If you did not intend to use a system-defined macro `major',
> >   you should #undef it after including .
> >   [-Werror=deprecated-declarations]
> >  if (virAsprintf(, "%d:%d ", major(sb.st_rdev), minor(sb.st_rdev)) 
> >< 0)
> >  ^~
> > In file included from /usr/include/features.h:397:0,
> >  from /usr/include/bits/libc-header-start.h:33,
> >  from /usr/include/stdio.h:28,
> >  from ../gnulib/lib/stdio.h:43,
> >  from util/vircgroup.c:26:
> > /usr/include/sys/sysmacros.h:87:1: note: declared here
> >  __SYSMACROS_DEFINE_MAJOR (__SYSMACROS_FST_IMPL_TEMPL)
> >  ^
> > 
> > Applications are supposed to use gnu_dev_major() or
> > gnu_dev_minor() respectively.
> 
> How does this work on non-gnu systems like *BSD / OS-X / Mingw[1] ?

It doesn't :)

  util/virutil.c:1667:16: error: implicit declaration of function
  'gnu_dev_major' is invalid in C99
  [-Werror,-Wimplicit-function-declaration]
*maj = gnu_dev_major(sb.st_rdev);
   ^
  util/virutil.c:1669:16: error: implicit declaration of function
  'gnu_dev_minor' is invalid in C99
  [-Werror,-Wimplicit-function-declaration]
*min = gnu_dev_minor(sb.st_rdev);
   ^

This is FreeBSD.

-- 
Andrea Bolognani / Red Hat / Virtualization

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

Re: [libvirt] [libvirt-php] libvirt_connect not reading out credential info on 0.5.2

[Michal Privoznik]

On 05.09.2016 23:08, Fernando Casas Schössow wrote:
> Hi again Michal,
> 
> I have more information to share.
> I think I can confirm that the patches are actually fixing the
> credentials problem and they are passed along.
> This is the content of test.log with libvirt-php output:
> 
> [2016-09-05 22:06:05 libvirt-php/core ]: libvirt_connect: credentials
> index 2
> [2016-09-05 22:06:05 libvirt-php/core ]: libvirt_connect: credentials
> index 5
> [2016-09-05 22:06:05 libvirt-php/core ]: libvirt_connect: Found 2
> elements for credentials
> [2016-09-05 22:06:05 libvirt-php/core ]: libvirt_virConnectAuthCallback:
> cred 0, type 5, prompt Enter Administrator's password for 192.168.7.2
> challenge 192.168.7.2
> [2016-09-05 22:06:05 libvirt-php/core ]: libvirt_virConnectAuthCallback:
> result somepass (16)
> 
> Of course then the process is crashing and the connection is aborted so
> I believe that the patches work fine (they fix the connection
> credentials not passed along issue) but once this is fixed the other
> problem is uncovered.

Cool, so at least that's confirmed. Can you please attach a debugger to
your php (or run php straight from it). Also, the stack trace is the
result of 'backtrace' command in gdb, not strace (which is a system call
tracer).

$ gdb $(which php)
(gdb) run test.php
(gbd) backtrace

And while you're at it:
$ valgrind --leak-check=full $(which php) test.php

Thank you.

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

Re: [libvirt] Nice golang bindings for libvirt

[Roman Mohr]

Hi Martin,

On Tue, Sep 6, 2016 at 11:30 AM, Martin Kletzander 
wrote:

> On Mon, Sep 05, 2016 at 05:44:03PM +0200, Roman Mohr wrote:
>
>> Hi all,
>>
>> I have been using these pretty nice libvirt-binding library for golang
>> [1].
>> It is under active development, pretty well maintained and has a very nice
>> travis setup to run tests against libvirt [2].
>>
>> I was wondering if the bindings could be added to [3] on libvirt.org.
>>
>>
> Hi, it's great to hear that.  We'd love to add those to the list.  Would
> you mind sending a patch to libvirt against 'docs/bindings.html.in'
> file?


Done [4].


>   That page needs heavy updating, but I can take care of the rest.
> If you don't feel like updating it, just let me know and I'll add it in
> while updating the old info that's there.
>
>
Thanks,

Roman


> Have a nice day,
> Martin
>
>
> Best Regards,
>> Roman
>>
>>
>> [1] https://github.com/rgbkrk
>> [2] https://github.com/rgbkrk/libvirt-go/blob/master/.travis.yml
>> [3] https://libvirt.org/bindings.html
>>
>
> [4]
https://www.redhat.com/archives/libvir-list/2016-September/msg00130.html


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

[libvirt] [PATCH] docs: Add libvirt-go Go bindings to binding page

[Roman Mohr]

Signed-off-by: Roman Mohr 
---
 docs/bindings.html.in | 4 
 1 file changed, 4 insertions(+)

diff --git a/docs/bindings.html.in b/docs/bindings.html.in
index 95cfe25..7fe26df 100644
--- a/docs/bindings.html.in
+++ b/docs/bindings.html.in
@@ -14,6 +14,10 @@
 C#: Arnaud Champion develops
 C# bindings.
   
+ 
+Go: Kyle Kelley et al. are developing
+https://github.com/rgbkrk/libvirt-go;>Go bindings.
+  
   
 Java: Daniel Veillard develops
 Java bindings.
-- 
2.5.5

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

Re: [libvirt] virt-admin commands aliases

[Martin Kletzander]

On Tue, Sep 06, 2016 at 09:02:19AM +0200, Erik Skultety wrote:

On 05/09/16 19:48, Daniel P. Berrange wrote:

On Mon, Sep 05, 2016 at 05:37:07PM +0200, Erik Skultety wrote:

Hi there,

after my presentation at KVM Forum, it was pointed out from the audience
that we might think about doing something about the naming of the
virt-admin's comands, since there is some sort of inconsistency: srv-
vs. client- vs. dmn- (not merged yet). When I sent patches to upstream I
already knew that the naming was not optimal, but I didn't come up with
anything better so I hoped that the reviewer might think of something
better which unfortunately did not happen.

Anyway, there are multiple options how this can be approached but I'm
not 100% satisfied with neither of them:

1) rename the commands completely
Although clean, obviously this is the non-preferred option because this
would break any backwards compatibility however, I think there is a fair
chance that people haven't actually started using it yet (although that
might change between 7.3 and 7.4).

2) create aliases for non-abbreviated forms of the commands
That way, srv- would become server- and dmn- would become daemon-.
However, by doing this we'll end up with 6 almost identical entries in
the commands structure which might be error-prone once we decide to
add/create a flag to the command primitive, since the flag would
have to be added both to the alias and to the original (unlikely, but
possible that someone might forget about that)

3) abbreviate client- to something like clnt-
Identical to the above except for the amount of duplicate entries which
would be reduced to 2

4) leave it as is if such a consensus is reached and accepted
I guess this does no need any additional comments.


I just vote for 4.

In retrospect it would have been nice to use 'server' instead of
'srv', but ultimately it isn't a functional problem.  The "solutions"
create extra code and/or inconsitency and/or break back-compat so just
aren't worth it IMHO.



Yeah, for me personally, it was either number 2 or 4 but as you write,
both of them suck in their own way and I just could not decide which one
sucked less.

Thanks for opinions guys, appreciated :)



I, honestly, would go with #2.  I know you know it, but to recapitulate;
we already have a wiring in the code for aliases, so if you change:

   {.name = "srv-list",
.handler = cmdSrvList,
.opts = NULL,
.info = info_srv_list,
.flags = 0
   },

to:
   {.name = "server-list",
.handler = cmdSrvList,
.opts = NULL,
.info = info_srv_list,
.flags = 0
   },
   {.name = "srv-list",
.handler = cmdSrvList,
.opts = NULL,
.info = info_srv_list,
.flags = VSH_CMD_FLAG_ALIAS
   },

You will have both commands, you will only see the 'server-list' in the
help and both will work.  The only thing you need to change is, that if
you add options to the commands that don't have any (yet), you need to
add the opts_srv_list to both commands.  But that's it.  For
srv-clients-list (BTW why isn't it named list-clients or client-list)
that would require no future change.  I know you mentioned that flag
changes would need to be done in both structures as well, but honestly,
thre's VSH_CMD_FLAG_NOCONNECT and VSH_CMD_FLAG_ALIAS.  And that's it.
And I don't think we'll have any new flags anytime soon.  I even dare to
say never.

But even if you don't like that, there are ways to mitigate even this
duplication.  Just make VSH_CMD_FLAG_ALIAS work as VSH_OT_ALIAS.  That
is just make the following work:

   {.name = "server-list",
.handler = cmdSrvList,
.opts = NULL,
.info = info_srv_list,
.flags = 0
   },
   {.name = "srv-list",
.flags = VSH_CMD_FLAG_ALIAS,
.aliased = "server-list"
   },

When parsing you'll see that it's an alias and return the opts, flags
and info for the original aliased command.  While on that, you can make
it even better; you can get rid of flags completely by just checking if
'aliased' is set.  If it is, just search for the original command and
that's it.

I'd vote for doing something like this and maybe keeping the aliases for
daemon and server added for new commands as well (that is when you add
server-make-me-a-sandwich, you also add srv- alias as well), but that's
not a hard requirement.

Just my €.02,
Martin


Erik


IOW, admit 'srv' sucks but don't change it, and ensure new server
commands continue to use 'srv' for consistency.

We can of couse use 'daemon-' as prefix for new commands, since we
have not yet released any versions using 'dmn-' as prefix


Regards,
Daniel



--
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] Nice golang bindings for libvirt

[Martin Kletzander]

On Mon, Sep 05, 2016 at 05:44:03PM +0200, Roman Mohr wrote:

Hi all,

I have been using these pretty nice libvirt-binding library for golang [1].
It is under active development, pretty well maintained and has a very nice
travis setup to run tests against libvirt [2].

I was wondering if the bindings could be added to [3] on libvirt.org.



Hi, it's great to hear that.  We'd love to add those to the list.  Would
you mind sending a patch to libvirt against 'docs/bindings.html.in'
file?  That page needs heavy updating, but I can take care of the rest.
If you don't feel like updating it, just let me know and I'll add it in
while updating the old info that's there.

Have a nice day,
Martin


Best Regards,
Roman


[1] https://github.com/rgbkrk
[2] https://github.com/rgbkrk/libvirt-go/blob/master/.travis.yml
[3] https://libvirt.org/bindings.html



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


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

Re: [libvirt] [libvirt-glib/libvirt-gconfig 17/17] gconfig, graphics: Avoid crash when gvir_config_object_new_from_xml() returns NULL

[Christophe Fergeau]

On Thu, Apr 21, 2016 at 01:05:45PM +0200, Christophe Fergeau wrote:
> This issues is more widespread than that, it would be better to fix it
> everywhere in one go (maybe through a gvir_config_object_check_type() or
> something like this?)

I believe this patch should do for now (with runtime warnings). Since
I'm not sure how widespread _new_from_xml() use is, maybe it's enough
for now, and we can decide on the best way to handle a NULL return when
we start getting runtime warnings from that patch?


From 0b428df2d82b6e669b50a3c400716c3883f9fe9c Mon Sep 17 00:00:00 2001
From: Christophe Fergeau 
Date: Tue, 6 Sep 2016 10:30:58 +0200
Subject: [libvirt-glib] gconfig: Add precondition to
 gvir_config_object_get_xml_node()

This will catch (among other things) cases when
gvir_config_object_get_xml_node() is called with a NULL argument. Not
catching this could cause a crash later on in cases when
gvir_config_object_new_from_xml() is called and returns NULL, and then
we call gvir_config_object_get_attribute() on it.
Now this should be caught with runtime warnings so that the underlying
issue can be fixed.
---
 libvirt-gconfig/libvirt-gconfig-object.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libvirt-gconfig/libvirt-gconfig-object.c 
b/libvirt-gconfig/libvirt-gconfig-object.c
index 6225de2..8cc4065 100644
--- a/libvirt-gconfig/libvirt-gconfig-object.c
+++ b/libvirt-gconfig/libvirt-gconfig-object.c
@@ -284,6 +284,8 @@ gvir_config_object_get_xml_doc(GVirConfigObject *config)
 G_GNUC_INTERNAL xmlNodePtr
 gvir_config_object_get_xml_node(GVirConfigObject *config)
 {
+g_return_val_if_fail(GVIR_CONFIG_IS_OBJECT(config), NULL);
+
 return config->priv->node;
 }
 
-- 
2.7.4


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

[libvirt] virsh blockcopy or virDomainBlockCopy auto converge

[Vasiliy Tolstov]

Does it possible to add something like auto converge in live migration
to blockcopy ?
In my case i'm try to do virsh blockcopy --pivot and sometime it fails
because domain write more data and it needs to be copied.

-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru

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

Re: [libvirt] virt-admin commands aliases

[Erik Skultety]

On 05/09/16 19:48, Daniel P. Berrange wrote:
> On Mon, Sep 05, 2016 at 05:37:07PM +0200, Erik Skultety wrote:
>> Hi there,
>>
>> after my presentation at KVM Forum, it was pointed out from the audience
>> that we might think about doing something about the naming of the
>> virt-admin's comands, since there is some sort of inconsistency: srv-
>> vs. client- vs. dmn- (not merged yet). When I sent patches to upstream I
>> already knew that the naming was not optimal, but I didn't come up with
>> anything better so I hoped that the reviewer might think of something
>> better which unfortunately did not happen.
>>
>> Anyway, there are multiple options how this can be approached but I'm
>> not 100% satisfied with neither of them:
>>
>> 1) rename the commands completely
>> Although clean, obviously this is the non-preferred option because this
>> would break any backwards compatibility however, I think there is a fair
>> chance that people haven't actually started using it yet (although that
>> might change between 7.3 and 7.4).
>>
>> 2) create aliases for non-abbreviated forms of the commands
>> That way, srv- would become server- and dmn- would become daemon-.
>> However, by doing this we'll end up with 6 almost identical entries in
>> the commands structure which might be error-prone once we decide to
>> add/create a flag to the command primitive, since the flag would
>> have to be added both to the alias and to the original (unlikely, but
>> possible that someone might forget about that)
>>
>> 3) abbreviate client- to something like clnt-
>> Identical to the above except for the amount of duplicate entries which
>> would be reduced to 2
>>
>> 4) leave it as is if such a consensus is reached and accepted
>> I guess this does no need any additional comments.
> 
> I just vote for 4.
> 
> In retrospect it would have been nice to use 'server' instead of
> 'srv', but ultimately it isn't a functional problem.  The "solutions"
> create extra code and/or inconsitency and/or break back-compat so just
> aren't worth it IMHO.
> 

Yeah, for me personally, it was either number 2 or 4 but as you write,
both of them suck in their own way and I just could not decide which one
sucked less.

Thanks for opinions guys, appreciated :)

Erik

> IOW, admit 'srv' sucks but don't change it, and ensure new server
> commands continue to use 'srv' for consistency.
> 
> We can of couse use 'daemon-' as prefix for new commands, since we
> have not yet released any versions using 'dmn-' as prefix
> 
> 
> Regards,
> Daniel
> 

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