Re: [libvirt] [PATCH v2] util: clang is failing to compile due to unused variables.
On 07/27/2018 05:17 PM, Julio Faracco wrote:
> After some recent patches, clang is throwing some errors related to
> unused variables. This is not happening when we use GCC with -Werror
> enabled. Only clang reports this warning.
>
> make[3]: Entering directory '/home/julio/Desktop/virt/libvirt/src'
> CC util/libvirt_util_la-virscsivhost.lo
> CC util/libvirt_util_la-virusb.lo
> CC util/libvirt_util_la-virmdev.lo
> util/virmdev.c:373:36: error: unused variable 'ret'
> [-Werror,-Wunused-variable]
> VIR_AUTOPTR(virMediatedDevice) ret = virMediatedDeviceListSteal(list,
> dev);
>^
> 1 error generated.
> Makefile:11579: recipe for target 'util/libvirt_util_la-virmdev.lo' failed
> make[3]: *** [util/libvirt_util_la-virmdev.lo] Error 1
> make[3]: *** Waiting for unfinished jobs
> util/virscsivhost.c:112:37: error: unused variable 'tmp'
> [-Werror,-Wunused-variable]
> VIR_AUTOPTR(virSCSIVHostDevice) tmp = virSCSIVHostDeviceListSteal(list,
> dev);
> ^
> 1 error generated.
> Makefile:11411: recipe for target 'util/libvirt_util_la-virscsivhost.lo'
> failed
> make[3]: *** [util/libvirt_util_la-virscsivhost.lo] Error 1
> util/virusb.c:511:31: error: unused variable 'ret' [-Werror,-Wunused-variable]
> VIR_AUTOPTR(virUSBDevice) ret = virUSBDeviceListSteal(list, dev);
>
> Signed-off-by: Julio Faracco
> ---
> src/util/virmdev.c | 2 +-
> src/util/virscsivhost.c | 2 +-
> src/util/virusb.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
Close, but you forgot something on each.
> diff --git a/src/util/virmdev.c b/src/util/virmdev.c
> index 4050835cc1..4492fd673e 100644
> --- a/src/util/virmdev.c
> +++ b/src/util/virmdev.c
> @@ -370,7 +370,7 @@ void
> virMediatedDeviceListDel(virMediatedDeviceListPtr list,
> virMediatedDevicePtr dev)
> {
> -VIR_AUTOPTR(virMediatedDevice) ret = virMediatedDeviceListSteal(list,
> dev);
> +virMediatedDeviceListSteal(list, dev);
Wrap a "virMediatedDeviceFree()" around this
> }
>
>
> diff --git a/src/util/virscsivhost.c b/src/util/virscsivhost.c
> index 280d0dc2fd..1a069e67ff 100644
> --- a/src/util/virscsivhost.c
> +++ b/src/util/virscsivhost.c
> @@ -109,7 +109,7 @@ void
> virSCSIVHostDeviceListDel(virSCSIVHostDeviceListPtr list,
>virSCSIVHostDevicePtr dev)
> {
> -VIR_AUTOPTR(virSCSIVHostDevice) tmp = virSCSIVHostDeviceListSteal(list,
> dev);
> +virSCSIVHostDeviceListSteal(list, dev);
Wrap a "virSCSIVHostDeviceFree()" around this.
> }
>
>
> diff --git a/src/util/virusb.c b/src/util/virusb.c
> index 609d54836f..d14b7623cc 100644
> --- a/src/util/virusb.c
> +++ b/src/util/virusb.c
> @@ -508,7 +508,7 @@ void
> virUSBDeviceListDel(virUSBDeviceListPtr list,
> virUSBDevicePtr dev)
> {
> -VIR_AUTOPTR(virUSBDevice) ret = virUSBDeviceListSteal(list, dev);
> +virUSBDeviceListSteal(list, dev);
Wrap a "virUSBDeviceFree()" around this.
I fixed those and pushed.
Tks,
John
> }
>
> virUSBDevicePtr
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/9] conf: Refactor virDomainCachetuneDefParse
On 2018年07月28日 00:58, John Ferlan wrote:
[...]
Secondary to that you've added a new error related to identical vcpus in
the parsing logic. Unfortunately that could make a domain disappear on a
libvirtd restart if for some reason it was already defined in that
manner. If there's a parsing error because two entries had identical
cpus, then there will be an error. Errors would cause a previously
OK/found domain to not be defined. So that type of check (now that the
Sorry, I am not sure if I understand correctly. Are you talking about
two domains(or VMs) with the same vcpus setting?
It's the:
+} else {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Identical vcpus in cachetunes found"));
+goto cleanup;
+}
that was new... Consider what would happen before any of these changes
were merged if that condition was true.
So I found tests/genericxml2xmlindata/cachetune-colliding-tunes.xml and
did a little test... Before your changes, the test fails with "error:
XML error: Overlapping vcpus in cachetunes", but with your changes the
test fails with "error: XML error: Identical vcpus in cachetunes found".
Yes! You are right. The error message is different.
So since both fail, that's good, no issue then. It was different which
is what drew my attention. In any case, just mention it in the commit
message that part of the change will "clarify" whether it's an overlap
or a redefinition.
Okay~, will update commit message in v2.
John
[...]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [RFC] secdrivers remembering original labels
Dear list, we have this very old bug [1] that I just keep pushing in front of me. I made several attempts to fix it. However, none of them made into the tree. I guess it's time to have discussion what to do about it. IIRC, the algorithm that I implemented last was to keep original label in XATTRs (among with some ref counter) and the last one to restore the label will look there and find the original label. There was a problem with two libvirtds fighting over a file on shared FS. So I guess my question is can we come up with a design that would work? Or at least work to the extent that we're satisfied with? Personally, I like the XATTRs approach. And to resolve the NFS race we can invent yet another lockspace to guard labelling - I also have a bug for that [2] (although, I'm not that familiar with lockspaces). I guess doing disk metadata locking is not going to be trivial, is it? Michal 1: https://bugzilla.redhat.com/show_bug.cgi?id=547546 2: https://bugzilla.redhat.com/show_bug.cgi?id=1524792 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] lxc: Use VIR_AUTOPTR for @veths in virLXCProcessStart
On Thu, Jul 26, 2018 at 05:36:29PM +0200, Michal Privoznik wrote: > Now that we have VIR_AUTOPTR and that @veths is a string list we > can use VIR_AUTOPTR to free it automagically. > > Signed-off-by: Michal Privoznik > --- Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] lxc: Turn @veths into a string list in virLXCProcessStart
On Thu, Jul 26, 2018 at 05:36:28PM +0200, Michal Privoznik wrote:
> This way it will be easier to use autofree.
>
> Signed-off-by: Michal Privoznik
> ---
> src/lxc/lxc_process.c | 24 +---
> 1 file changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
> index d021a890f7..3ac39d598c 100644
> --- a/src/lxc/lxc_process.c
> +++ b/src/lxc/lxc_process.c
> @@ -514,8 +514,7 @@ static int virLXCProcessSetupNamespaces(virConnectPtr
> conn,
> * virLXCProcessSetupInterfaces:
> * @conn: pointer to connection
> * @def: pointer to virtual machine structure
> - * @nveths: number of interfaces
> - * @veths: interface names
> + * @veths: string list of interface names
> *
> * Sets up the container interfaces by creating the veth device pairs and
> * attaching the parent end to the appropriate bridge. The container end
> @@ -525,7 +524,6 @@ static int virLXCProcessSetupNamespaces(virConnectPtr
> conn,
> */
> static int virLXCProcessSetupInterfaces(virConnectPtr conn,
> virDomainDefPtr def,
> -size_t *nveths,
> char ***veths)
> {
> int ret = -1;
> @@ -534,6 +532,8 @@ static int virLXCProcessSetupInterfaces(virConnectPtr
> conn,
> virDomainNetDefPtr net;
> virDomainNetType type;
>
> +*veths = NULL;
> +
> for (i = 0; i < def->nnets; i++) {
> char *veth = NULL;
> virNetDevBandwidthPtr actualBandwidth;
> @@ -549,9 +549,6 @@ static int virLXCProcessSetupInterfaces(virConnectPtr
> conn,
> if (virDomainNetAllocateActualDevice(def, net) < 0)
> goto cleanup;
>
> -if (VIR_EXPAND_N(*veths, *nveths, 1) < 0)
> -goto cleanup;
Contrary to what I said in the previous patch, I hadn't realized we were
expanding a list by 1 in a loop before I looked at this patch. That is very
inefficient and we'll keep doing that. Now I'm biased towards tracking the size
of the list so that we can do VIR_EXPAND_N(*veths, 0, def->nnets) and then just
add new elements, of course that would require tweaking the virStringListAdd
more.
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC 00/39] qemu: Add support for -blockdev
Am 27.07.2018 um 10:01 hat Peter Krempa geschrieben:
> On Thu, Jul 26, 2018 at 17:04:19 +0800, Fam Zheng wrote:
> > On Thu, 07/26 10:44, Kevin Wolf wrote:
> > > Am 25.07.2018 um 17:57 hat Peter Krempa geschrieben:
> > > > This series adds support for starting and hotplug of disks with
> > > > -blockdev/blockdev-add.
> > > >
> > > > Blockjobs are not supported and thus the last patch should not be
> > > > applied yet as some refactoring of the jobs is required.
> > > >
> > > > At the beginning of the series there are a few cleanup patches which may
> > > > be pushed even at this point.
> > > >
> > > > The main reason this is in RFC state is that block stats reporting does
> > > > not work.
> > > >
> > > > The following command:
> > > >
> > > > {"execute":"query-blockstats","arguments":{"query-nodes":true}}
> > >
> > > query-nodes was added in commit f71eaa74c0b by Fam and Max, CCed. I'm
> > > not sure what it was needed for at all and the commit message doesn't
> > > help with that, but I suppose the addition was related to
> > > wr_highest_offset (see below).
> >
> > Yes, this was part of RHBZ 1158094. Sorry about the poor commit message.
>
> Well the problem is that when using -blockdev with 'query-nodes' false/not
> present you get a
> even more useless output:
>
> virsh qemu-monitor-command --pretty upstream
> '{"execute":"query-blockstats","arguments":{"query-nodes":false}}'
> {
> "return": [
>
> ],
> "id": "libvirt-20"
> }
That's a problem. Apparently it ignores anonymous BlockBackends, which
it shouldn't do. I'll look into that.
> 'query-named-block-nodes' don't provide the 'stats' section either:
> (other drives snipped)
[...]
> Thus libvirt can't provide the stats it used to with -drive. Since we
> need to special-case the code for gathering stats for -blockdev it
> should not be a problem if e.g. query-named-block-nodes reported the
> stats section along. (perhaps with a boolean flag to do so which also
> could disable the nesting)
The only one it could theoretically provide is wr_highest_offset, the
other ones simply don't exist at the node level. And this is probably
not the only one you're interested in.
> In either case we need the stats in the same extent as we had before
> with -drive and query-blockstats.
Yes, that's clear.
Kevin
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 RESEND 09/12] qemu: Generate and use zPCI device in QEMU command line
在 2018/7/24 下午11:09, Andrea Bolognani 写道:
On Tue, 2018-07-10 at 16:02 +0800, Yi Min Zhao wrote:
[...]
+bool
+qemuCheckDeviceIsZPCI(virDomainDeviceInfoPtr info)
+{
+if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI &&
+((info->pciAddressExtFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI) ||
+ info->addr.pci.zpci))
+return true;
Missing curly braces.
Also, do you really need to check both the flags and the presence
of the zPCI address bits? It looks like either one or the other
should be enough or, if that's not the case, it should be made so
because having to check for two separate conditions makes me feel
like it would introduce bugs in the long run.
This is actually a problem. I add info->addr.pci.zpci check in order to
check
if the user specifies zpci address in xml even though it has no zpci
support.
The callers of this function checks zpci capability. If no zpci cap but
the user
specfies zpci address, report an error.
I will change the logic and remove the check on info->addr.pci.zpci.
[...]
+char *qemuBuildZPCIDevStr(virDomainDeviceInfoPtr dev);
+
+bool qemuCheckDeviceIsZPCI(virDomainDeviceInfoPtr info);
Is this really necessary? Can't these two functions be static?
They are also used in qemu_hotplug.c file.
[...]
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1014,6 +1014,8 @@ mymain(void)
QEMU_CAPS_CCW, QEMU_CAPS_VIRTIO_S390);
DO_TEST("disk-virtio-scsi-ccw", QEMU_CAPS_VIRTIO_SCSI,
QEMU_CAPS_CCW, QEMU_CAPS_VIRTIO_S390);
+DO_TEST("disk-virtio-s390-zpci", QEMU_CAPS_DEVICE_ZPCI,
+QEMU_CAPS_CCW, QEMU_CAPS_VIRTIO_S390);
DO_TEST("disk-order",
QEMU_CAPS_DRIVE_BOOT, QEMU_CAPS_VIRTIO_BLK_SCSI);
DO_TEST("disk-virtio-drive-queues",
@@ -1574,6 +1576,18 @@ mymain(void)
QEMU_CAPS_DEVICE_VFIO_PCI);
DO_TEST_PARSE_ERROR("hostdev-mdev-invalid-target-address",
QEMU_CAPS_DEVICE_VFIO_PCI);
+DO_TEST("hostdev-vfio-zpci",
+QEMU_CAPS_DEVICE_VFIO_PCI, QEMU_CAPS_DEVICE_ZPCI);
+DO_TEST("hostdev-vfio-zpci-multidomain-many",
+QEMU_CAPS_DEVICE_VFIO_PCI, X_QEMU_CAPS_HOST_PCI_MULTIDOMAIN,
Capabilities with X_QEMU prefix are no longer used, so you should
not list them here.
Sure.
+QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_ZPCI);
+DO_TEST("hostdev-vfio-zpci-autogenerate",
+QEMU_CAPS_DEVICE_VFIO_PCI, QEMU_CAPS_DEVICE_ZPCI);
+DO_TEST("hostdev-vfio-zpci-boundaries",
+QEMU_CAPS_DEVICE_VFIO_PCI, X_QEMU_CAPS_HOST_PCI_MULTIDOMAIN,
+QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_ZPCI);
+DO_TEST_FAILURE("hostdev-vfio-zpci",
+QEMU_CAPS_DEVICE_VFIO_PCI);
Please add these to qemuxml2xmltest at the same time.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC 00/39] qemu: Add support for -blockdev
On Thu, Jul 26, 2018 at 17:04:19 +0800, Fam Zheng wrote:
> On Thu, 07/26 10:44, Kevin Wolf wrote:
> > Am 25.07.2018 um 17:57 hat Peter Krempa geschrieben:
> > > This series adds support for starting and hotplug of disks with
> > > -blockdev/blockdev-add.
> > >
> > > Blockjobs are not supported and thus the last patch should not be
> > > applied yet as some refactoring of the jobs is required.
> > >
> > > At the beginning of the series there are a few cleanup patches which may
> > > be pushed even at this point.
> > >
> > > The main reason this is in RFC state is that block stats reporting does
> > > not work.
> > >
> > > The following command:
> > >
> > > {"execute":"query-blockstats","arguments":{"query-nodes":true}}
> >
> > query-nodes was added in commit f71eaa74c0b by Fam and Max, CCed. I'm
> > not sure what it was needed for at all and the commit message doesn't
> > help with that, but I suppose the addition was related to
> > wr_highest_offset (see below).
>
> Yes, this was part of RHBZ 1158094. Sorry about the poor commit message.
Well the problem is that when using -blockdev with 'query-nodes' false/not
present you get a
even more useless output:
virsh qemu-monitor-command --pretty upstream
'{"execute":"query-blockstats","arguments":{"query-nodes":false}}'
{
"return": [
],
"id": "libvirt-20"
}
'query-named-block-nodes' don't provide the 'stats' section either:
(other drives snipped)
{
"iops_rd": 0,
"detect_zeroes": "off",
"image": {
"virtual-size": 520032256,
"filename": "/var/lib/libvirt/images/systemrescuecd-x86-4.9.5.iso",
"format": "raw",
"actual-size": 520036352,
"dirty-flag": false
},
"iops_wr": 0,
"ro": true,
"node-name": "libvirt-7-format",
"backing_file_depth": 0,
"drv": "raw",
"iops": 0,
"bps_wr": 0,
"write_threshold": 0,
"encrypted": false,
"bps": 0,
"bps_rd": 0,
"cache": {
"no-flush": false,
"direct": false,
"writeback": true
},
"file": "/var/lib/libvirt/images/systemrescuecd-x86-4.9.5.iso",
"encryption_key_missing": false
},
{
"iops_rd": 0,
"detect_zeroes": "off",
"image": {
"virtual-size": 520032256,
"filename": "/var/lib/libvirt/images/systemrescuecd-x86-4.9.5.iso",
"format": "file",
"actual-size": 520036352,
"dirty-flag": false
},
"iops_wr": 0,
"ro": true,
"node-name": "libvirt-7-storage",
"backing_file_depth": 0,
"drv": "file",
"iops": 0,
"bps_wr": 0,
"write_threshold": 0,
"encrypted": false,
"bps": 0,
"bps_rd": 0,
"cache": {
"no-flush": false,
"direct": false,
"writeback": true
},
"file": "/var/lib/libvirt/images/systemrescuecd-x86-4.9.5.iso",
"encryption_key_missing": false
}
],
"id": "libvirt-19"
}
Thus libvirt can't provide the stats it used to with -drive. Since we
need to special-case the code for gathering stats for -blockdev it
should not be a problem if e.g. query-named-block-nodes reported the
stats section along. (perhaps with a boolean flag to do so which also
could disable the nesting)
In either case we need the stats in the same extent as we had before
with -drive and query-blockstats.
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] util: Rework virStringListAdd
On Thu, Jul 26, 2018 at 05:36:27PM +0200, Michal Privoznik wrote:
> So every caller does the same: they use virStringListAdd() to add
^This sounds a bit like there's a handful of them ;).
> new item into the list and then free the old copy to replace it
> with new list. It's not very memory effective, nor environmental
> friendly.
>
> Signed-off-by: Michal Privoznik
> ---
> src/util/virmacmap.c | 8 ++--
> src/util/virstring.c | 34 ++
> src/util/virstring.h | 4 ++--
> tests/virstringtest.c | 6 +-
> 4 files changed, 19 insertions(+), 33 deletions(-)
>
> diff --git a/src/util/virmacmap.c b/src/util/virmacmap.c
> index 88ca9b3f36..c7b700fa05 100644
> --- a/src/util/virmacmap.c
> +++ b/src/util/virmacmap.c
> @@ -90,7 +90,6 @@ virMacMapAddLocked(virMacMapPtr mgr,
> {
> int ret = -1;
> char **macsList = NULL;
> -char **newMacsList = NULL;
>
> if ((macsList = virHashLookup(mgr->macs, domain)) &&
> virStringListHasString((const char**) macsList, mac)) {
> @@ -98,15 +97,12 @@ virMacMapAddLocked(virMacMapPtr mgr,
> goto cleanup;
> }
>
> -if (!(newMacsList = virStringListAdd((const char **) macsList, mac)) ||
> -virHashUpdateEntry(mgr->macs, domain, newMacsList) < 0)
> +if (virStringListAdd(, mac) < 0 ||
> +virHashUpdateEntry(mgr->macs, domain, macsList) < 0)
> goto cleanup;
> -newMacsList = NULL;
> -virStringListFree(macsList);
>
> ret = 0;
> cleanup:
> -virStringListFree(newMacsList);
> return ret;
> }
>
> diff --git a/src/util/virstring.c b/src/util/virstring.c
> index 93fda69d7f..59f57a97b8 100644
> --- a/src/util/virstring.c
> +++ b/src/util/virstring.c
> @@ -175,32 +175,26 @@ char *virStringListJoin(const char **strings,
> * @strings: a NULL-terminated array of strings
> * @item: string to add
> *
> - * Creates new strings list with all strings duplicated and @item
> - * at the end of the list. Callers is responsible for freeing
> - * both @strings and returned list.
> + * Appends @item into string list @strings. If *@strings is not
> + * allocated yet new string list is created.
> + *
> + * Returns: 0 on success,
> + * -1 otherwise
> */
> -char **
> -virStringListAdd(const char **strings,
> +int
> +virStringListAdd(char ***strings,
> const char *item)
> {
> -char **ret = NULL;
> -size_t i = virStringListLength(strings);
> +size_t i = virStringListLength((const char **) *strings);
>
> -if (VIR_ALLOC_N(ret, i + 2) < 0)
> -goto error;
This scales linearly, but given the number of "every" caller of this and the
projected frequency of usage, I don't really care about N sentinels.
You could consider VIR_EXPAND_N to get rid of the explicit sentinel assignment
below, your call.
> +if (VIR_REALLOC_N(*strings, i + 2) < 0)
> +return -1;
>
> -for (i = 0; strings && strings[i]; i++) {
> -if (VIR_STRDUP(ret[i], strings[i]) < 0)
> -goto error;
> -}
> +(*strings)[i + 1] = NULL;
> +if (VIR_STRDUP((*strings)[i], item) < 0)
> +return -1;
>
> -if (VIR_STRDUP(ret[i], item) < 0)
> -goto error;
> -
> -return ret;
> - error:
> -virStringListFree(ret);
> -return NULL;
> +return 0;
Reviewed-by: Erik Skultety
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] secdrivers remembering original labels
On Fri, Jul 27, 2018 at 09:56:23AM +0200, Michal Privoznik wrote: > Dear list, > > we have this very old bug [1] that I just keep pushing in front of me. I > made several attempts to fix it. However, none of them made into the > tree. I guess it's time to have discussion what to do about it. IIRC, > the algorithm that I implemented last was to keep original label in > XATTRs (among with some ref counter) and the last one to restore the > label will look there and find the original label. There was a problem > with two libvirtds fighting over a file on shared FS. IIRC there was a second problem around security of XATTRs. eg, if we set an XATTR it was possible for QEMU to change it once we given QEMU privs on the image. Thus a malicious QEMU can cause libvirtd to change the image to an arbitrary user on shutdown. I think it was possible to deal with that on Linux, because the kernel hsa some xattr namespacing that is reserved for only root. This protection is worthless though when using NFS images as you can't trust the remote NFS client to honour the same restriction. > So I guess my question is can we come up with a design that would work? > Or at least work to the extent that we're satisfied with? > > Personally, I like the XATTRs approach. And to resolve the NFS race we > can invent yet another lockspace to guard labelling - I also have a bug > for that [2] (although, I'm not that familiar with lockspaces). I guess > doing disk metadata locking is not going to be trivial, is it? Yeah, for sure we need two distinct locking regimes. Our current locking aims to protect disk content, and the lifetime of the lock is the entire duration of the QEMU process. For XATTRs, we only need write protection for the short time window in which the security drivers execute, which is at start, during hotplug/unplug, during shutdown, and finally migration. I think we could achieve this with the virtlockd if we make it use the same locking file, but a different byte offset within the file. Just need to make sure it doesn't clash with the byte offset QEMU has chosen, nor the current offset we use. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] secdrivers remembering original labels
On 07/27/2018 11:01 AM, Daniel P. Berrangé wrote: > On Fri, Jul 27, 2018 at 09:56:23AM +0200, Michal Privoznik wrote: >> Dear list, >> >> we have this very old bug [1] that I just keep pushing in front of me. I >> made several attempts to fix it. However, none of them made into the >> tree. I guess it's time to have discussion what to do about it. IIRC, >> the algorithm that I implemented last was to keep original label in >> XATTRs (among with some ref counter) and the last one to restore the >> label will look there and find the original label. There was a problem >> with two libvirtds fighting over a file on shared FS. > > IIRC there was a second problem around security of XATTRs. eg, if we set > an XATTR it was possible for QEMU to change it once we given QEMU privs > on the image. Thus a malicious QEMU can cause libvirtd to change the > image to an arbitrary user on shutdown. > > I think it was possible to deal with that on Linux, because the kernel > hsa some xattr namespacing that is reserved for only root. Yes, there are basically 4 levels (defined by prefix of attribute name): security. system. trusted. user. For the first two there is no restriction on side of VFS (but there might be some coming from underlying FS and/or security module). The trusted. can be get/set only by CAP_SYS_ADMIN process. And finally user. can be set only one regular files (plus some other restrictions), but it's available for basically everybody. IIRC, I used trusted. namespace because of CAP_SYS_ADMIN. Nota bene, if a file is stored on NFS and only CAP_SYS_ADMIN can change the attribute's value they are CAP_SYS_ADMIN already - they might change seclabel too. Why bother mangling libvirt's records. IOW, if somebody runs qemu with CAP_SYS_ADMIN all bets are off anyway. This would of course mean that there's no label restore for session daemon, but that can hardly ever work. Do we even initialize DAC/SELinux drivers for session daemon? > This protection > is worthless though when using NFS images as you can't trust the remote NFS > client to honour the same restriction. > >> So I guess my question is can we come up with a design that would work? >> Or at least work to the extent that we're satisfied with? >> >> Personally, I like the XATTRs approach. And to resolve the NFS race we >> can invent yet another lockspace to guard labelling - I also have a bug >> for that [2] (although, I'm not that familiar with lockspaces). I guess >> doing disk metadata locking is not going to be trivial, is it? > > Yeah, for sure we need two distinct locking regimes. Our current locking > aims to protect disk content, and the lifetime of the lock is the entire > duration of the QEMU process. For XATTRs, we only need write protection > for the short time window in which the security drivers execute, which > is at start, during hotplug/unplug, during shutdown, and finally migration. I guess migration would be covered by start. Since these locks would be held only for very short period (basically they would be acquired just before we try to set seclabel and released after disk content lock is successfully acquired) they can be exclusive. However, since we want to protect more than just disk seclabels, we need to acquire this new lock from more places. > > I think we could achieve this with the virtlockd if we make it use the > same locking file, but a different byte offset within the file. Just > need to make sure it doesn't clash with the byte offset QEMU has chosen, > nor the current offset we use. Makes sense. So the first step is to introduce metadata locking. I'll look into that. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] secdrivers remembering original labels
On Fri, Jul 27, 2018 at 01:27:40PM +0200, Michal Privoznik wrote: > On 07/27/2018 11:01 AM, Daniel P. Berrangé wrote: > > On Fri, Jul 27, 2018 at 09:56:23AM +0200, Michal Privoznik wrote: > >> Dear list, > >> > >> we have this very old bug [1] that I just keep pushing in front of me. I > >> made several attempts to fix it. However, none of them made into the > >> tree. I guess it's time to have discussion what to do about it. IIRC, > >> the algorithm that I implemented last was to keep original label in > >> XATTRs (among with some ref counter) and the last one to restore the > >> label will look there and find the original label. There was a problem > >> with two libvirtds fighting over a file on shared FS. > > > > IIRC there was a second problem around security of XATTRs. eg, if we set > > an XATTR it was possible for QEMU to change it once we given QEMU privs > > on the image. Thus a malicious QEMU can cause libvirtd to change the > > image to an arbitrary user on shutdown. > > > > I think it was possible to deal with that on Linux, because the kernel > > hsa some xattr namespacing that is reserved for only root. > > Yes, there are basically 4 levels (defined by prefix of attribute name): > > security. > system. > trusted. > user. > > For the first two there is no restriction on side of VFS (but there > might be some coming from underlying FS and/or security module). The > trusted. can be get/set only by CAP_SYS_ADMIN process. And finally user. > can be set only one regular files (plus some other restrictions), but > it's available for basically everybody. > > IIRC, I used trusted. namespace because of CAP_SYS_ADMIN. Nota bene, if > a file is stored on NFS and only CAP_SYS_ADMIN can change the > attribute's value they are CAP_SYS_ADMIN already - they might change > seclabel too. Why bother mangling libvirt's records. IOW, if somebody > runs qemu with CAP_SYS_ADMIN all bets are off anyway. Yep, if QEMU has CAP_SYS_ADMIN they're doomed no matter what. > > This would of course mean that there's no label restore for session > daemon, but that can hardly ever work. Do we even initialize DAC/SELinux > drivers for session daemon? The DAC driver doesn't work for session daemons (since you can't setuid obviously), but the sVirt driver *does* work. > > This protection > > is worthless though when using NFS images as you can't trust the remote NFS > > client to honour the same restriction. > > > >> So I guess my question is can we come up with a design that would work? > >> Or at least work to the extent that we're satisfied with? > >> > >> Personally, I like the XATTRs approach. And to resolve the NFS race we > >> can invent yet another lockspace to guard labelling - I also have a bug > >> for that [2] (although, I'm not that familiar with lockspaces). I guess > >> doing disk metadata locking is not going to be trivial, is it? > > > > Yeah, for sure we need two distinct locking regimes. Our current locking > > aims to protect disk content, and the lifetime of the lock is the entire > > duration of the QEMU process. For XATTRs, we only need write protection > > for the short time window in which the security drivers execute, which > > is at start, during hotplug/unplug, during shutdown, and finally migration. > > I guess migration would be covered by start. Since these locks would be > held only for very short period (basically they would be acquired just > before we try to set seclabel and released after disk content lock is > successfully acquired) they can be exclusive. > > However, since we want to protect more than just disk seclabels, we need > to acquire this new lock from more places. > > > > > I think we could achieve this with the virtlockd if we make it use the > > same locking file, but a different byte offset within the file. Just > > need to make sure it doesn't clash with the byte offset QEMU has chosen, > > nor the current offset we use. > > Makes sense. So the first step is to introduce metadata locking. I'll > look into that. Yes, in fact the metdata locking is desirable even ignoring the original task of restoring original disk labels. The metadata locking alone would better protect against startup races between system libvirtds accessing the same NFS. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 10/41] util: cgroup: use VIR_AUTOPTR for aggregate types
On Tue, Jul 24, 2018 at 09:22:11PM +0530, Sukrit Bhatnagar wrote:
> By making use of GNU C's cleanup attribute handled by the
> VIR_AUTOPTR macro for declaring aggregate pointer variables,
> majority of the calls to *Free functions can be dropped, which
> in turn leads to getting rid of most of our cleanup sections.
>
> Signed-off-by: Sukrit Bhatnagar
> Reviewed-by: Erik Skultety
> ---
...
> @@ -1277,10 +1268,10 @@ virCgroupNewPartition(const char *path,
>int controllers,
>virCgroupPtr *group)
> {
> -int ret = -1;
> VIR_AUTOFREE(char *) parentPath = NULL;
> VIR_AUTOFREE(char *) newPath = NULL;
> -virCgroupPtr parent = NULL;
> +VIR_AUTOPTR(virCgroup) parent = NULL;
> +VIR_AUTOPTR(virCgroup) tmpGroup = NULL;
> VIR_DEBUG("path=%s create=%d controllers=%x",
>path, create, controllers);
>
> @@ -1315,12 +1306,11 @@ virCgroupNewPartition(const char *path,
> }
> }
>
> -ret = 0;
> +return 0;
> +
> cleanup:
> -if (ret != 0)
> -virCgroupFree(*group);
> -virCgroupFree(parent);
> -return ret;
> +VIR_STEAL_PTR(tmpGroup, *group);
> +return -1;
^Not exactly what I had in mind, we're still touching the caller provided data
too much. Have a look at this diff:
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 61fafe26f8..472a8167f5 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -1283,34 +1283,31 @@ virCgroupNewPartition(const char *path,
}
if (virCgroupSetPartitionSuffix(path, ) < 0)
-goto cleanup;
+return -1;
-if (virCgroupNew(-1, newPath, NULL, controllers, group) < 0)
-goto cleanup;
+if (virCgroupNew(-1, newPath, NULL, controllers, ) < 0)
+return -1;
if (STRNEQ(newPath, "/")) {
char *tmp;
if (VIR_STRDUP(parentPath, newPath) < 0)
-goto cleanup;
+return -1;
tmp = strrchr(parentPath, '/');
tmp++;
*tmp = '\0';
if (virCgroupNew(-1, parentPath, NULL, controllers, ) < 0)
-goto cleanup;
+return -1;
-if (virCgroupMakeGroup(parent, *group, create, VIR_CGROUP_NONE) < 0) {
-virCgroupRemove(*group);
-goto cleanup;
+if (virCgroupMakeGroup(parent, tmpGroup, create, VIR_CGROUP_NONE) < 0)
{
+virCgroupRemove(tmpGroup);
+return -1;
}
}
+VIR_STEAL_PTR(*group, tmpGroup);
return 0;
-
- cleanup:
-VIR_STEAL_PTR(tmpGroup, *group);
-return -1;
After ^this, we'll only touch the caller provided data once we're sure nothing
can go wrong anymore. I'll squash that in.
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC 0/2] util: Add VIR_ENUM_IMPL_LABEL
On 07/26/2018 07:49 PM, Cole Robinson wrote: > This series adds VIR_ENUM_IMPL_LABEL and a demo conversion. > > VIR_ENUM_IMPL_LABEL allows passing in a string that briefly > describes the value the enum represents, which we use to > generate error messages for FromString and ToString > function failures. > > This will allow us to drop a lot of code and unique strings > that need translating. > > Patch 2 is an example usage, converting virDomainVirtType > enum. It's not a full conversion for this particular enum, > just a demo. The enum creation now looks like > > VIR_ENUM_IMPL_LABEL(virDomainVirt, > "domain type", > VIR_DOMAIN_VIRT_LAST, ... > > FromString failure reports this error for value '' > > invalid argument: Unknown 'domain type' value '' > > ToString failure reports this error for unknown type=83 > > internal error: Unknown 'domain type' internal value '83' > > > Seems like a win to me but I'm interested in other opinions. > This could also be a good BiteSizedTask for converting existing > enums > Agreed. > Cole Robinson (2): > util: Add VIR_ENUM_IMPL_LABEL > conf: Convert virDomainVirtType to VIR_ENUM_IMPL_LABEL > > src/conf/domain_conf.c | 10 ++ > src/util/virutil.c | 20 > src/util/virutil.h | 15 ++- > 3 files changed, 28 insertions(+), 17 deletions(-) > I like this. You can count on my ACK. But we should probably let others chime in and express their preference. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC 1/2] util: Add VIR_ENUM_IMPL_LABEL
On 07/26/2018 07:49 PM, Cole Robinson wrote:
> This allows passing in a string label describing the enum, which can
> be used to autogenerate error messages
>
> Signed-off-by: Cole Robinson
> ---
> src/util/virutil.c | 20
> src/util/virutil.h | 15 ++-
> 2 files changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/src/util/virutil.c b/src/util/virutil.c
> index a908422feb..6d23a26a74 100644
> --- a/src/util/virutil.c
> +++ b/src/util/virutil.c
> @@ -444,16 +444,22 @@ virParseVersionString(const char *str, unsigned long
> *version,
>
> int virEnumFromString(const char *const*types,
>unsigned int ntypes,
> - const char *type)
> + const char *type,
> + const char * const label)
> {
> size_t i;
> if (!type)
> -return -1;
> +goto error;
>
> for (i = 0; i < ntypes; i++)
> if (STREQ(types[i], type))
> return i;
>
> + error:
> +if (label) {
If we rewrite all of our enums into _LABEL then @label is always going
to be non-NULL. But we need this check for now. We are not there yet.
> +virReportError(VIR_ERR_INVALID_ARG,
> + _("Unknown '%s' value '%s'"), label, type);
@type can be NULL, therefore NULLSTR(type).
> +}
> return -1;
> }
>
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt-dbus build for virt-preview
On Fri, Jul 27, 2018 at 12:38:47PM +0300, Vasiliy Tolstov wrote: > Hi! Does it possible to add libvirt-dbus building for virt-preview repo? > My use case - get latest libvirt-dbus in current fedora stable release. Hi, I guess that we can do that, in the future it will be definitely useful, but for now I'm planning to update libvirt-dbus in stable releases as well. Currently libvirt-dbus requires libvirt-3.0.0, once the required version is higher than the libvirt in one of the stable releases I'll stop updating libvirt-dbus. Libvirt-dbus is currently under development in a way that it's still catching with all the libvirt APIs. The strategy is to implement all reasonable APIs up to libvirt-3.0.0, after that there will be separate release of libvirt-dbus to gradually cover libvirt APIs in order to make downstream maintainers life easier so they can pick specific libvirt-dbus version which will work with the shipped libvirt with all APIs covered. Pavel signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] libvirt-dbus build for virt-preview
Hi! Does it possible to add libvirt-dbus building for virt-preview repo? My use case - get latest libvirt-dbus in current fedora stable release. -- 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] [PATCH RFC 0/2] util: Add VIR_ENUM_IMPL_LABEL
On Fri, Jul 27, 2018 at 11:17:13AM +0200, Michal Privoznik wrote: > On 07/26/2018 07:49 PM, Cole Robinson wrote: > > This series adds VIR_ENUM_IMPL_LABEL and a demo conversion. > > > > VIR_ENUM_IMPL_LABEL allows passing in a string that briefly > > describes the value the enum represents, which we use to > > generate error messages for FromString and ToString > > function failures. > > > > This will allow us to drop a lot of code and unique strings > > that need translating. > > > > Patch 2 is an example usage, converting virDomainVirtType > > enum. It's not a full conversion for this particular enum, > > just a demo. The enum creation now looks like > > > > VIR_ENUM_IMPL_LABEL(virDomainVirt, > > "domain type", > > VIR_DOMAIN_VIRT_LAST, ... > > > > FromString failure reports this error for value '' > > > > invalid argument: Unknown 'domain type' value '' > > > > ToString failure reports this error for unknown type=83 > > > > internal error: Unknown 'domain type' internal value '83' > > > > > > Seems like a win to me but I'm interested in other opinions. > > This could also be a good BiteSizedTask for converting existing > > enums > > > > Agreed. > > > Cole Robinson (2): > > util: Add VIR_ENUM_IMPL_LABEL > > conf: Convert virDomainVirtType to VIR_ENUM_IMPL_LABEL > > > > src/conf/domain_conf.c | 10 ++ > > src/util/virutil.c | 20 > > src/util/virutil.h | 15 ++- > > 3 files changed, 28 insertions(+), 17 deletions(-) > > > > I like this. You can count on my ACK. But we should probably let others > chime in and express their preference. I think its good. My only comment is whether we could come up with a way to gradually convert each file, while still finishing up with VIR_ENUM_IMPL as the macro name. A mass rename at the end is one option, but perhaps theres a more clever approach ? Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/4] Documentation/vfio-mediated-device.txt: update for aggregation attribute
On Fri, 27 Jul 2018 10:16:58 +0800 Zhenyu Wang wrote: > On 2018.07.26 17:46:40 +0200, Cornelia Huck wrote: > > On Fri, 20 Jul 2018 10:19:28 +0800 > > Zhenyu Wang wrote: > > > > > Update mdev doc on new aggregration attribute and instances attribute > > > for mdev. > > > > > > Cc: Kirti Wankhede > > > Cc: Alex Williamson > > > Cc: Kevin Tian > > > Signed-off-by: Zhenyu Wang > > > --- > > > Documentation/vfio-mediated-device.txt | 39 ++ > > > 1 file changed, 33 insertions(+), 6 deletions(-) > > > > > > diff --git a/Documentation/vfio-mediated-device.txt > > > b/Documentation/vfio-mediated-device.txt > > > index c3f69bcaf96e..9ec9495dcbe7 100644 > > > --- a/Documentation/vfio-mediated-device.txt > > > +++ b/Documentation/vfio-mediated-device.txt > > > @@ -211,12 +211,20 @@ Directories and files under the sysfs for Each > > > Physical Device > > >| | |--- description > > >| | |--- [devices] > > >| |--- [] > > > - | |--- create > > > - | |--- name > > > - | |--- available_instances > > > - | |--- device_api > > > - | |--- description > > > - | |--- [devices] > > > + | ||--- create > > > + | ||--- name > > > + | ||--- available_instances > > > + | ||--- device_api > > > + | ||--- description > > > + | ||--- [devices] > > > + | |--- [] > > > + | ||--- create > > > + | ||--- name > > > + | ||--- available_instances > > > + | ||--- device_api > > > + | ||--- description > > > + | ||--- > > > + | ||--- [devices] > > > > > > * [mdev_supported_types] > > > > > > @@ -260,6 +268,19 @@ Directories and files under the sysfs for Each > > > Physical Device > > >This attribute should show brief features/description of the type. > > > This is > > >optional attribute. > > > > > > +* > > > + > > > + The description is to show feature for one instance of the type. > > > > > > > You are talking about "one instance" here. Can this be different for > > the same type with different physical devices? > > > > I would expect for normal mdev types, driver might expose like x2, x4, x8 > types > which split hw resource equally. But for type with aggregation feature, it can > set user wanted number of instances. Sorry maybe my use of word was not > clear, how > about "one example of type"? Maybe my question was confusing as well... is an attribute that is exposed for a particular type under a particular physical device. - If is always the same for that particular type, regardless of which physical device we're dealing with, let's just drop the "one instance" sentence. - If it instead depends on what physical device we're handling, I'd write something like "The contents of this attribute depend both on the type and on the particular instance." -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dbus PATCH 3/4] tests: Don't open-code node_device_create()
The fixture is pretty trivial right now, so open-coding
it is not a big deal; however, we're going to add more
logic to it soon and we definitely don't want to end up
having to worry about duplicated code.
Signed-off-by: Andrea Bolognani
---
tests/test_nodedev.py | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tests/test_nodedev.py b/tests/test_nodedev.py
index c68cb66..ce70dfc 100755
--- a/tests/test_nodedev.py
+++ b/tests/test_nodedev.py
@@ -3,7 +3,6 @@
import dbus
import libvirttest
import pytest
-import xmldata
class TestNodeDevice(libvirttest.BaseTestClass):
@@ -30,7 +29,7 @@ class TestNodeDevice(libvirttest.BaseTestClass):
self.connect.connect_to_signal('NodeDeviceEvent', node_device_created)
-path =
self.connect.NodeDeviceCreateXML(xmldata.minimal_node_device_xml, 0)
+path = self.node_device_create()
assert isinstance(path, dbus.ObjectPath)
self.main_loop()
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dbus PATCH 0/4] Fix 'make check' with libvirt 3.0.0
Andrea Bolognani (4): tests: Drop pytest.mark.usefixtures from test_nodedev tests: Move some test cases to test_nodedev tests: Don't open-code node_device_create() tests: Make node_device_create() work with libvirt 3.0.0 tests/libvirttest.py | 17 - tests/test_connect.py | 27 --- tests/test_nodedev.py | 26 +- tests/xmldata.py | 2 +- 4 files changed, 42 insertions(+), 30 deletions(-) -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dbus PATCH 4/4] tests: Make node_device_create() work with libvirt 3.0.0
The scsi_host2 nodedev, which our test nodedev uses as its
parent, was added to the test driver with commit 5c2ff641e10c,
which is included in libvirt 3.1.0; before that, there was a
similar device called test-scsi-host-vport, which is no longer
present after libvirt 3.0.0.
Since there's no nodedev that we can use as parent both with
libvirt 3.0.0 and with later versions, our only option is to
perform a lookup at runtime and use whatever's available.
Signed-off-by: Andrea Bolognani
---
tests/libvirttest.py | 17 -
tests/xmldata.py | 2 +-
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/tests/libvirttest.py b/tests/libvirttest.py
index 776b12d..14baf5b 100644
--- a/tests/libvirttest.py
+++ b/tests/libvirttest.py
@@ -91,7 +91,22 @@ class BaseTestClass():
This fixture should be used in the setup of every test manipulating
with node devices.
"""
-path =
self.connect.NodeDeviceCreateXML(xmldata.minimal_node_device_xml, 0)
+# We need a usable parent nodedev: possible candidates are
+# scsi_host2 (available since libvirt 3.1.0) and
+# test-scsi-host-vport (available until libvirt 3.0.0).
+#
+# Looking up a non-existing nodedev raises an exception, so
+# we need to ignore them here: that's okay, because if we
+# can't find any then NodeDeviceCreateXML() itself will fail
+xml = xmldata.minimal_node_device_xml
+for parent in ['scsi_host2', 'test-scsi-host-vport']:
+try:
+if self.connect.NodeDeviceLookupByName(parent):
+xml = xml.replace('@parent@', parent)
+break
+except dbus.exceptions.DBusException:
+pass
+path = self.connect.NodeDeviceCreateXML(xml, 0)
return path
@pytest.fixture
diff --git a/tests/xmldata.py b/tests/xmldata.py
index a70bac1..8075052 100644
--- a/tests/xmldata.py
+++ b/tests/xmldata.py
@@ -41,7 +41,7 @@ minimal_network_xml = '''
minimal_node_device_xml = '''
scsi_host22
- scsi_host2
+ @parent@
22
22
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dbus PATCH 2/4] tests: Move some test cases to test_nodedev
For whatever reason, a few nodedev-related test cases
have ended up in test_connect instead of the more
appropriate test_nodedev. Move them.
Signed-off-by: Andrea Bolognani
---
tests/test_connect.py | 27 ---
tests/test_nodedev.py | 26 ++
2 files changed, 26 insertions(+), 27 deletions(-)
diff --git a/tests/test_connect.py b/tests/test_connect.py
index f481356..9cc51db 100755
--- a/tests/test_connect.py
+++ b/tests/test_connect.py
@@ -161,19 +161,6 @@ class TestConnect(libvirttest.BaseTestClass):
self.main_loop()
-@pytest.mark.usefixtures("node_device_create")
-@pytest.mark.parametrize("lookup_method_name,lookup_item", [
-("NodeDeviceLookupByName", 'Name'),
-])
-def test_connect_node_device_lookup_by_property(self, lookup_method_name,
lookup_item, node_device_create):
-"""Parameterized test for all NodeDeviceLookupBy* API calls of Connect
interface
-"""
-original_path = node_device_create
-obj = self.bus.get_object('org.libvirt', original_path)
-prop = obj.Get('org.libvirt.NodeDevice', lookup_item,
dbus_interface=dbus.PROPERTIES_IFACE)
-path = getattr(self.connect, lookup_method_name)(prop)
-assert original_path == path
-
@pytest.mark.parametrize("lookup_method_name,lookup_item", [
("NetworkLookupByName", 'Name'),
("NetworkLookupByUUID", 'UUID'),
@@ -186,20 +173,6 @@ class TestConnect(libvirttest.BaseTestClass):
path = getattr(self.connect, lookup_method_name)(prop)
assert original_path == path
-def test_connect_node_device_create_xml(self):
-def node_device_created(path, event, _detail):
-if event != libvirttest.NodeDeviceEvent.CREATED:
-return
-assert isinstance(path, dbus.ObjectPath)
-self.loop.quit()
-
-self.connect.connect_to_signal('NodeDeviceEvent', node_device_created)
-
-path =
self.connect.NodeDeviceCreateXML(xmldata.minimal_node_device_xml, 0)
-assert isinstance(path, dbus.ObjectPath)
-
-self.main_loop()
-
def test_connect_node_get_cpu_stats(self):
stats = self.connect.NodeGetCPUStats(0, 0)
assert isinstance(stats, dbus.Dictionary)
diff --git a/tests/test_nodedev.py b/tests/test_nodedev.py
index 082cf0b..c68cb66 100755
--- a/tests/test_nodedev.py
+++ b/tests/test_nodedev.py
@@ -3,11 +3,37 @@
import dbus
import libvirttest
import pytest
+import xmldata
class TestNodeDevice(libvirttest.BaseTestClass):
""" Tests for methods and properties of the NodeDevice interface
"""
+@pytest.mark.parametrize("lookup_method_name,lookup_item", [
+("NodeDeviceLookupByName", 'Name'),
+])
+def test_connect_node_device_lookup_by_property(self, lookup_method_name,
lookup_item, node_device_create):
+"""Parameterized test for all NodeDeviceLookupBy* API calls of Connect
interface
+"""
+original_path = node_device_create
+obj = self.bus.get_object('org.libvirt', original_path)
+prop = obj.Get('org.libvirt.NodeDevice', lookup_item,
dbus_interface=dbus.PROPERTIES_IFACE)
+path = getattr(self.connect, lookup_method_name)(prop)
+assert original_path == path
+
+def test_connect_node_device_create(self):
+def node_device_created(path, event, _detail):
+if event != libvirttest.NodeDeviceEvent.CREATED:
+return
+assert isinstance(path, dbus.ObjectPath)
+self.loop.quit()
+
+self.connect.connect_to_signal('NodeDeviceEvent', node_device_created)
+
+path =
self.connect.NodeDeviceCreateXML(xmldata.minimal_node_device_xml, 0)
+assert isinstance(path, dbus.ObjectPath)
+
+self.main_loop()
def test_node_device_destroy(self, node_device_create):
def node_device_deleted(path, event, _detail):
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dbus PATCH 1/4] tests: Drop pytest.mark.usefixtures from test_nodedev
We're already using node_device_create explicitly for all
test cases in order to retrieve the result of the fixture,
so using pytest.mark.usefixtures as well is pointless.
Additionally, we're soon going to add some test cases
where we need the fixture *not* to run automatically.
Signed-off-by: Andrea Bolognani
---
tests/test_nodedev.py | 1 -
1 file changed, 1 deletion(-)
diff --git a/tests/test_nodedev.py b/tests/test_nodedev.py
index 22c8d60..082cf0b 100755
--- a/tests/test_nodedev.py
+++ b/tests/test_nodedev.py
@@ -5,7 +5,6 @@ import libvirttest
import pytest
-@pytest.mark.usefixtures("node_device_create")
class TestNodeDevice(libvirttest.BaseTestClass):
""" Tests for methods and properties of the NodeDevice interface
"""
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] news: Usb and sata for virsh attach-disk --address
On 07/26/2018 03:33 PM, Han Han wrote: > Signed-off-by: Han Han > --- > docs/news.xml | 10 ++ > 1 file changed, 10 insertions(+) ACKed and pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/4] New mdev type handling for aggregated resources
On Thu, Jul 26, 2018 at 06:04:10PM +0200, Cornelia Huck wrote: > On Thu, 26 Jul 2018 17:43:45 +0200 > Erik Skultety wrote: > > > On Thu, Jul 26, 2018 at 05:30:07PM +0200, Cornelia Huck wrote: > > > One thing I noticed is that we have seem to have an optional (?) > > > vendor-driver created "aggregation" attribute (which always prints > > > "true" in the Intel driver). Would it be better or worse for libvirt if > > > it contained some kind of upper boundary or so? Additionally, would it > > > > Can you be more specific? Although, I wouldn't argue against data that > > conveys > > some information, since I would treat the mere presence of the optional > > attribute as a supported feature that we can expose. Therefore, additional > > *structured* data which sets clear limits to a certain feature is only a > > plus > > that we can expose to the users/management layer. > > My question is what would be easiest for libvirt: > > - "aggregation" attribute only present when driver supports aggregation > (possibly containing max number of resources to be aggregated) > - "aggregation" attribute always present; contains '1' if driver does > not support aggregation and 'm' if driver can aggregate 'm' resources Both are fine from libvirt's POV, but IMHO the former makes a bit more sense and I'm in favour of that one, IOW the presence of an attribute denotes a new functionality which we can report, if it's missing, the feature is clearly lacking- I don't think we (libvirt) should be reporting the value 1 explicitly in the XML if the feature is missing, we can assume 1 as the default. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] lxc: Turn @veths into a string list in virLXCProcessStart
On Fri, Jul 27, 2018 at 02:34:41PM +0200, Michal Privoznik wrote:
> On 07/27/2018 10:37 AM, Erik Skultety wrote:
> > On Thu, Jul 26, 2018 at 05:36:28PM +0200, Michal Privoznik wrote:
> >> This way it will be easier to use autofree.
> >>
> >> Signed-off-by: Michal Privoznik
> >> ---
> >> src/lxc/lxc_process.c | 24 +---
> >> 1 file changed, 9 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
> >> index d021a890f7..3ac39d598c 100644
> >> --- a/src/lxc/lxc_process.c
> >> +++ b/src/lxc/lxc_process.c
> >> @@ -514,8 +514,7 @@ static int virLXCProcessSetupNamespaces(virConnectPtr
> >> conn,
> >> * virLXCProcessSetupInterfaces:
> >> * @conn: pointer to connection
> >> * @def: pointer to virtual machine structure
> >> - * @nveths: number of interfaces
> >> - * @veths: interface names
> >> + * @veths: string list of interface names
> >> *
> >> * Sets up the container interfaces by creating the veth device pairs and
> >> * attaching the parent end to the appropriate bridge. The container end
> >> @@ -525,7 +524,6 @@ static int virLXCProcessSetupNamespaces(virConnectPtr
> >> conn,
> >> */
> >> static int virLXCProcessSetupInterfaces(virConnectPtr conn,
> >> virDomainDefPtr def,
> >> -size_t *nveths,
> >> char ***veths)
> >> {
> >> int ret = -1;
> >> @@ -534,6 +532,8 @@ static int virLXCProcessSetupInterfaces(virConnectPtr
> >> conn,
> >> virDomainNetDefPtr net;
> >> virDomainNetType type;
> >>
> >> +*veths = NULL;
> >> +
> >> for (i = 0; i < def->nnets; i++) {
> >> char *veth = NULL;
> >> virNetDevBandwidthPtr actualBandwidth;
> >> @@ -549,9 +549,6 @@ static int virLXCProcessSetupInterfaces(virConnectPtr
> >> conn,
> >> if (virDomainNetAllocateActualDevice(def, net) < 0)
> >> goto cleanup;
> >>
> >> -if (VIR_EXPAND_N(*veths, *nveths, 1) < 0)
> >> -goto cleanup;
> >
> > Contrary to what I said in the previous patch, I hadn't realized we were
> > expanding a list by 1 in a loop before I looked at this patch. That is very
> > inefficient and we'll keep doing that. Now I'm biased towards tracking the
> > size
> > of the list so that we can do VIR_EXPAND_N(*veths, 0, def->nnets) and then
> > just
> > add new elements, of course that would require tweaking the virStringListAdd
> > more.
> >
>
> Hold on. I'm not quite sure I follow. Are you suggesting to pre-allocate
> the string list and ditch virStringListAdd completely? Something like
> this?
Yes, that's the idea. Originally, I didn't think dropping virStringListAdd would
be necessary, but it's clear from the snippet below that it would.
Erik
>
> diff --git i/src/lxc/lxc_process.c w/src/lxc/lxc_process.c
> index 6eef17d1ce..33c806630b 100644
> --- i/src/lxc/lxc_process.c
> +++ w/src/lxc/lxc_process.c
> @@ -532,7 +532,8 @@ static int virLXCProcessSetupInterfaces(virConnectPtr
> conn,
> virDomainNetDefPtr net;
> virDomainNetType type;
>
> -*veths = NULL;
> +if (VIR_ALLOC_N(*veths, def->nnets + 1) < 0)
> +return -1;
>
> for (i = 0; i < def->nnets; i++) {
> char *veth = NULL;
> @@ -601,8 +602,7 @@ static int virLXCProcessSetupInterfaces(virConnectPtr
> conn,
> }
> }
>
> -if (virStringListAdd(veths, veth) < 0)
> -goto cleanup;
> +(*veths)[i] = veth;
>
> if (VIR_STRDUP(def->nets[i]->ifname_guest_actual, veth) < 0)
> goto cleanup;
>
>
> Michal
>
> --
> 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] [dbus PATCH] gitignore: Ignore Vim swap files
Signed-off-by: Andrea Bolognani --- Pushed as trivial. .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index e89a5d3..b4abf66 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ *.log *.o *.pyc +*.swp *.trs *Makefile *Makefile.in -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 37/41] util: iscsi: use VIR_AUTOPTR for aggregate types
On Tue, Jul 24, 2018 at 09:22:38PM +0530, Sukrit Bhatnagar wrote: > By making use of GNU C's cleanup attribute handled by the > VIR_AUTOPTR macro for declaring aggregate pointer variables, > majority of the calls to *Free functions can be dropped, which > in turn leads to getting rid of most of our cleanup sections. > > Signed-off-by: Sukrit Bhatnagar > Reviewed-by: Erik Skultety For the time being, I'm going to retract my ^RB (the same goes for the previous patch). There are some merge conflicts while applying this and the previous patch (there's been a work on this module recently), so you should resolve them a sent as part of the next batch. Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] lxc: Turn @veths into a string list in virLXCProcessStart
On Fri, Jul 27, 2018 at 05:02:36PM +0200, Erik Skultety wrote:
> On Fri, Jul 27, 2018 at 02:34:41PM +0200, Michal Privoznik wrote:
> > On 07/27/2018 10:37 AM, Erik Skultety wrote:
> > > On Thu, Jul 26, 2018 at 05:36:28PM +0200, Michal Privoznik wrote:
> > >> This way it will be easier to use autofree.
> > >>
> > >> Signed-off-by: Michal Privoznik
> > >> ---
> > >> src/lxc/lxc_process.c | 24 +---
> > >> 1 file changed, 9 insertions(+), 15 deletions(-)
> > >>
> > >> diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
> > >> index d021a890f7..3ac39d598c 100644
> > >> --- a/src/lxc/lxc_process.c
> > >> +++ b/src/lxc/lxc_process.c
> > >> @@ -514,8 +514,7 @@ static int
> > >> virLXCProcessSetupNamespaces(virConnectPtr conn,
> > >> * virLXCProcessSetupInterfaces:
> > >> * @conn: pointer to connection
> > >> * @def: pointer to virtual machine structure
> > >> - * @nveths: number of interfaces
> > >> - * @veths: interface names
> > >> + * @veths: string list of interface names
> > >> *
> > >> * Sets up the container interfaces by creating the veth device pairs
> > >> and
> > >> * attaching the parent end to the appropriate bridge. The container
> > >> end
> > >> @@ -525,7 +524,6 @@ static int
> > >> virLXCProcessSetupNamespaces(virConnectPtr conn,
> > >> */
> > >> static int virLXCProcessSetupInterfaces(virConnectPtr conn,
> > >> virDomainDefPtr def,
> > >> -size_t *nveths,
> > >> char ***veths)
> > >> {
> > >> int ret = -1;
> > >> @@ -534,6 +532,8 @@ static int
> > >> virLXCProcessSetupInterfaces(virConnectPtr conn,
> > >> virDomainNetDefPtr net;
> > >> virDomainNetType type;
> > >>
> > >> +*veths = NULL;
> > >> +
> > >> for (i = 0; i < def->nnets; i++) {
> > >> char *veth = NULL;
> > >> virNetDevBandwidthPtr actualBandwidth;
> > >> @@ -549,9 +549,6 @@ static int
> > >> virLXCProcessSetupInterfaces(virConnectPtr conn,
> > >> if (virDomainNetAllocateActualDevice(def, net) < 0)
> > >> goto cleanup;
> > >>
> > >> -if (VIR_EXPAND_N(*veths, *nveths, 1) < 0)
> > >> -goto cleanup;
> > >
> > > Contrary to what I said in the previous patch, I hadn't realized we were
> > > expanding a list by 1 in a loop before I looked at this patch. That is
> > > very
> > > inefficient and we'll keep doing that. Now I'm biased towards tracking
> > > the size
> > > of the list so that we can do VIR_EXPAND_N(*veths, 0, def->nnets) and
> > > then just
> > > add new elements, of course that would require tweaking the
> > > virStringListAdd
> > > more.
> > >
> >
> > Hold on. I'm not quite sure I follow. Are you suggesting to pre-allocate
> > the string list and ditch virStringListAdd completely? Something like
> > this?
>
> Yes, that's the idea. Originally, I didn't think dropping virStringListAdd
> would
> be necessary, but it's clear from the snippet below that it would.
>
> Erik
Something I forgot:
Reviewed-by: Erik Skultety
>
> >
> > diff --git i/src/lxc/lxc_process.c w/src/lxc/lxc_process.c
> > index 6eef17d1ce..33c806630b 100644
> > --- i/src/lxc/lxc_process.c
> > +++ w/src/lxc/lxc_process.c
> > @@ -532,7 +532,8 @@ static int virLXCProcessSetupInterfaces(virConnectPtr
> > conn,
> > virDomainNetDefPtr net;
> > virDomainNetType type;
> >
> > -*veths = NULL;
> > +if (VIR_ALLOC_N(*veths, def->nnets + 1) < 0)
> > +return -1;
> >
> > for (i = 0; i < def->nnets; i++) {
> > char *veth = NULL;
> > @@ -601,8 +602,7 @@ static int virLXCProcessSetupInterfaces(virConnectPtr
> > conn,
> > }
> > }
> >
> > -if (virStringListAdd(veths, veth) < 0)
> > -goto cleanup;
> > +(*veths)[i] = veth;
> >
> > if (VIR_STRDUP(def->nets[i]->ifname_guest_actual, veth) < 0)
> > goto cleanup;
> >
> >
> > Michal
> >
> > --
> > 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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 RESEND 09/12] qemu: Generate and use zPCI device in QEMU command line
On Fri, 2018-07-27 at 15:53 +0800, Yi Min Zhao wrote:
> 在 2018/7/24 下午11:09, Andrea Bolognani 写道:
> > Also, do you really need to check both the flags and the presence
> > of the zPCI address bits? It looks like either one or the other
> > should be enough or, if that's not the case, it should be made so
> > because having to check for two separate conditions makes me feel
> > like it would introduce bugs in the long run.
>
> This is actually a problem. I add info->addr.pci.zpci check in order to
> check
> if the user specifies zpci address in xml even though it has no zpci
> support.
> The callers of this function checks zpci capability. If no zpci cap but
> the user
> specfies zpci address, report an error.
>
> I will change the logic and remove the check on info->addr.pci.zpci.
Yeah, you definitely want to report an error if the QEMU binary
doesn't support zPCI or the user is attempting to do something
silly like add zPCI-related information to devices attached to
an x86_64 guest...
> > > +char *qemuBuildZPCIDevStr(virDomainDeviceInfoPtr dev);
> > > +
> > > +bool qemuCheckDeviceIsZPCI(virDomainDeviceInfoPtr info);
> >
> > Is this really necessary? Can't these two functions be static?
>
> They are also used in qemu_hotplug.c file.
Right, I overlooked that :)
That said, they aren't used in the hotplug code until the next
patch, so it would IMHO make sense to define them as static in
this patch and export them in the next one, at the same time as
you actually start using them outside of the file.
[...]
> > > +DO_TEST("hostdev-vfio-zpci",
> > > +QEMU_CAPS_DEVICE_VFIO_PCI, QEMU_CAPS_DEVICE_ZPCI);
> > > +DO_TEST("hostdev-vfio-zpci-multidomain-many",
> > > +QEMU_CAPS_DEVICE_VFIO_PCI, X_QEMU_CAPS_HOST_PCI_MULTIDOMAIN,
> >
> > Capabilities with X_QEMU prefix are no longer used, so you should
> > not list them here.
>
> Sure.
I forgot to mention, please have a single capability per line and
make sure the set of capabilities used in xml2xml and xml2argv is
exactly the same.
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 25/41] util: usb: use VIR_AUTOPTR for aggregate types
On Tue, Jul 24, 2018 at 09:22:26PM +0530, Sukrit Bhatnagar wrote:
> By making use of GNU C's cleanup attribute handled by the
> VIR_AUTOPTR macro for declaring aggregate pointer variables,
> majority of the calls to *Free functions can be dropped, which
> in turn leads to getting rid of most of our cleanup sections.
>
> Signed-off-by: Sukrit Bhatnagar
> ---
> src/util/virusb.c | 15 ---
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/src/util/virusb.c b/src/util/virusb.c
> index 47b407b..df94959 100644
> --- a/src/util/virusb.c
> +++ b/src/util/virusb.c
> @@ -123,8 +123,10 @@ virUSBDeviceSearch(unsigned int vendor,
> bool found = false;
> char *ignore = NULL;
> struct dirent *de;
> -virUSBDeviceListPtr list = NULL, ret = NULL;
> -virUSBDevicePtr usb;
> +virUSBDeviceListPtr list = NULL;
> +virUSBDeviceListPtr ret = NULL;
> +VIR_AUTOPTR(virUSBDevice) usb = NULL;
> +virUSBDevicePtr *ptr = NULL;
> int direrr;
>
> if (!(list = virUSBDeviceListNew()))
> @@ -173,13 +175,13 @@ virUSBDeviceSearch(unsigned int vendor,
> }
>
> usb = virUSBDeviceNew(found_bus, found_devno, vroot);
> +ptr =
I don't see a reason for the @ptr variable, you can work with @usb directly,
I'll make the change before pushing.
Reviewed-by: Erik Skultety
> +
> if (!usb)
> goto cleanup;
>
> -if (virUSBDeviceListAdd(list, ) < 0) {
> -virUSBDeviceFree(usb);
> +if (virUSBDeviceListAdd(list, ptr) < 0)
> goto cleanup;
> -}
>
> if (found)
> break;
> @@ -508,8 +510,7 @@ void
> virUSBDeviceListDel(virUSBDeviceListPtr list,
> virUSBDevicePtr dev)
> {
> -virUSBDevicePtr ret = virUSBDeviceListSteal(list, dev);
> -virUSBDeviceFree(ret);
> +VIR_AUTOPTR(virUSBDevice) ret = virUSBDeviceListSteal(list, dev);
> }
>
> virUSBDevicePtr
> --
> 1.8.3.1
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH 0/4] Fix 'make check' with libvirt 3.0.0
On Fri, Jul 27, 2018 at 02:37:37PM +0200, Andrea Bolognani wrote: > Andrea Bolognani (4): > tests: Drop pytest.mark.usefixtures from test_nodedev > tests: Move some test cases to test_nodedev > tests: Don't open-code node_device_create() > tests: Make node_device_create() work with libvirt 3.0.0 > > tests/libvirttest.py | 17 - > tests/test_connect.py | 27 --- > tests/test_nodedev.py | 26 +- > tests/xmldata.py | 2 +- > 4 files changed, 42 insertions(+), 30 deletions(-) > > -- > 2.17.1 > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list Please remove the second patch moving the test file - see reasoning there. So the third patch should be adjusted but in the original file, test_connect.py With these fixed: Reviewed-by: Katerina Koukiou signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 22/41] util: usb: modify virUSBDeviceListAdd to take double pointer
On Tue, Jul 24, 2018 at 09:22:23PM +0530, Sukrit Bhatnagar wrote: > Modify virUSBDeviceListAdd to take a double pointer to > virUSBDevicePtr as the second argument. This will enable usage > of cleanup macros upon the virUSBDevicePtr item which is to be > added to the list as it will be cleared by virInsertElementsN > upon success. > > Signed-off-by: Sukrit Bhatnagar > --- Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 28/41] util: scsi: use VIR_AUTOPTR for aggregate types
On Tue, Jul 24, 2018 at 09:22:29PM +0530, Sukrit Bhatnagar wrote:
> By making use of GNU C's cleanup attribute handled by the
> VIR_AUTOPTR macro for declaring aggregate pointer variables,
> majority of the calls to *Free functions can be dropped, which
> in turn leads to getting rid of most of our cleanup sections.
>
> Signed-off-by: Sukrit Bhatnagar
> Reviewed-by: Erik Skultety
> ---
> src/util/virscsi.c | 37 +++--
> 1 file changed, 15 insertions(+), 22 deletions(-)
>
> diff --git a/src/util/virscsi.c b/src/util/virscsi.c
> index ba0a688..f97f6b5 100644
> --- a/src/util/virscsi.c
> +++ b/src/util/virscsi.c
> @@ -189,7 +189,8 @@ virSCSIDeviceNew(const char *sysfs_prefix,
> bool readonly,
> bool shareable)
> {
> -virSCSIDevicePtr dev, ret = NULL;
> +VIR_AUTOPTR(virSCSIDevice) dev = NULL;
> +virSCSIDevicePtr ret = NULL;
> VIR_AUTOFREE(char *) sg = NULL;
> VIR_AUTOFREE(char *) vendor_path = NULL;
> VIR_AUTOFREE(char *) model_path = NULL;
> @@ -207,46 +208,43 @@ virSCSIDeviceNew(const char *sysfs_prefix,
> dev->shareable = shareable;
>
> if (!(sg = virSCSIDeviceGetSgName(prefix, adapter, bus, target, unit)))
> -goto cleanup;
> +return NULL;
>
> if (virSCSIDeviceGetAdapterId(adapter, >adapter) < 0)
> -goto cleanup;
> +return NULL;
>
> if (virAsprintf(>name, "%d:%u:%u:%llu", dev->adapter,
> dev->bus, dev->target, dev->unit) < 0 ||
> virAsprintf(>sg_path, "%s/%s",
> sysfs_prefix ? sysfs_prefix : "/dev", sg) < 0)
> -goto cleanup;
> +return NULL;
>
> if (!virFileExists(dev->sg_path)) {
> virReportSystemError(errno,
> _("SCSI device '%s': could not access %s"),
> dev->name, dev->sg_path);
> -goto cleanup;
> +return NULL;
> }
>
> if (virAsprintf(_path,
> "%s/%s/vendor", prefix, dev->name) < 0 ||
> virAsprintf(_path,
> "%s/%s/model", prefix, dev->name) < 0)
> -goto cleanup;
> +return NULL;
>
> if (virFileReadAll(vendor_path, 1024, ) < 0)
> -goto cleanup;
> +return NULL;
>
> if (virFileReadAll(model_path, 1024, ) < 0)
> -goto cleanup;
> +return NULL;
>
> virTrimSpaces(vendor, NULL);
> virTrimSpaces(model, NULL);
>
> if (virAsprintf(>id, "%s:%s", vendor, model) < 0)
> -goto cleanup;
> +return NULL;
>
> -ret = dev;
> - cleanup:
> -if (!ret)
> -virSCSIDeviceFree(dev);
> +VIR_STEAL_PTR(ret, dev);
> return ret;
> }
>
> @@ -281,21 +279,17 @@ virSCSIDeviceSetUsedBy(virSCSIDevicePtr dev,
> const char *drvname,
> const char *domname)
> {
> -virUsedByInfoPtr copy;
> +VIR_AUTOPTR(virUsedByInfo) copy = NULL;
I'll add a new line ^here before merging.
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Plans for next release
So we are getting very close to the end of the month, I suggest to enter freeze tomorrow morning, then plan for an RC2 on Tuesday, that way if everythiong goes well next release would be on Thursday 2nd Aug, I hope this is fine with everyone's schedule, thanks, Daniel -- Daniel Veillard | Red Hat Developers Tools http://developer.redhat.com/ veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 00/41] use GNU C's cleanup attribute in src/util (batch II)
On Tue, Jul 24, 2018 at 09:22:01PM +0530, Sukrit Bhatnagar wrote: > This second series of patches also modifies a few files in src/util > to use VIR_AUTOFREE and VIR_AUTOPTR for automatic freeing of memory > and get rid of some VIR_FREE macro invocations and *Free function > calls. > > The argument type of virCgroupFree is changed from virCgroupPtr * > to virCgroupPtr and that of virUSBDeviceListAdd is changed to take > a double pointer to virUSBDevice. So I tweaked a few patches as noted in my review and pushed the series. I left out the iscsi patches as there were some merge conflicts that need to be resolved. Thanks, Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 1/3] qemuxml2argvtest: Set more fake drivers
On Fri, Jul 27, 2018 at 05:24:45PM +0200, Michal Privoznik wrote: > So far we are setting only fake secret and storage drivers. > Therefore if the code wants to call a public NWFilter API (like > qemuBuildInterfaceCommandLine() and qemuBuildNetCommandLine() are > doing) the virGetConnectNWFilter() function will try to actually > spawn session daemon because there's no connection object set to > handle NWFilter driver. > > Even though I haven't experienced the same problem with the rest > of the drivers (interface, network and node dev), the reasoning > above can be applied to them as well. > > At the same time, now that connection object is registered for > the drivers, the public APIs will throw > virReportUnsupportedError(). And since we don't provide any error > func the error is printed to stderr. Fix this by setting dummy > error func. Is this last paragraph stale ? you're not seeing a dummy error func in this patch ... > > Signed-off-by: Michal Privoznik > --- > tests/qemuxml2argvtest.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c > index 84117a3e63..8901c7bde4 100644 > --- a/tests/qemuxml2argvtest.c > +++ b/tests/qemuxml2argvtest.c > @@ -471,6 +471,10 @@ testCompareXMLToArgv(const void *data) > conn->secretDriver = > conn->storageDriver = > > +virSetConnectInterface(conn); > +virSetConnectNetwork(conn); > +virSetConnectNWFilter(conn); > +virSetConnectNodeDev(conn); > virSetConnectSecret(conn); > virSetConnectStorage(conn); > > -- > 2.16.4 > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virpci: Drop unused @ret in virPCIDeviceListDel
So after 00dc991ca16730 the function is one line long and the
line is declaring a variable which is never used in fact. Replace
it with actual free() call instead of autofree.
Signed-off-by: Michal Privoznik
---
Pushed under build breaker rule as this is causing clang to complain.
Huh, who uses that? :-P
src/util/virpci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 634df8d35f..6cf2acf2d1 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -2003,7 +2003,7 @@ void
virPCIDeviceListDel(virPCIDeviceListPtr list,
virPCIDevicePtr dev)
{
-VIR_AUTOPTR(virPCIDevice) ret = virPCIDeviceListSteal(list, dev);
+virPCIDeviceFree(virPCIDeviceListSteal(list, dev));
}
int
--
2.16.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 7/9] conf: Refactor virDomainCachetuneDefParse
[...]
>>
>> Secondary to that you've added a new error related to identical vcpus in
>> the parsing logic. Unfortunately that could make a domain disappear on a
>> libvirtd restart if for some reason it was already defined in that
>> manner. If there's a parsing error because two entries had identical
>> cpus, then there will be an error. Errors would cause a previously
>> OK/found domain to not be defined. So that type of check (now that the
>
> Sorry, I am not sure if I understand correctly. Are you talking about
> two domains(or VMs) with the same vcpus setting?
It's the:
+} else {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Identical vcpus in cachetunes found"));
+goto cleanup;
+}
that was new... Consider what would happen before any of these changes
were merged if that condition was true.
So I found tests/genericxml2xmlindata/cachetune-colliding-tunes.xml and
did a little test... Before your changes, the test fails with "error:
XML error: Overlapping vcpus in cachetunes", but with your changes the
test fails with "error: XML error: Identical vcpus in cachetunes found".
So since both fail, that's good, no issue then. It was different which
is what drew my attention. In any case, just mention it in the commit
message that part of the change will "clarify" whether it's an overlap
or a redefinition.
John
[...]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 3/3] virtestmock: Track action
As advertised in the previous commit, we need the list of
accessed files to also contain action that caused the $path to
appear on the list. Not only this enables us to fine tune our
white list rules it also helps us to see why $path is reported.
For instance:
/run/user/1000/libvirt/libvirt-sock: connect: qemuxml2argvtest: QEMU
XML-2-ARGV net-vhostuser-multiq
Signed-off-by: Michal Privoznik
---
tests/virtestmock.c | 39 ++-
1 file changed, 22 insertions(+), 17 deletions(-)
diff --git a/tests/virtestmock.c b/tests/virtestmock.c
index 654af24a10..25aadf8aea 100644
--- a/tests/virtestmock.c
+++ b/tests/virtestmock.c
@@ -88,7 +88,8 @@ static void init_syms(void)
}
static void
-printFile(const char *file)
+printFile(const char *file,
+ const char *func)
{
FILE *fp;
const char *testname = getenv("VIR_TEST_MOCK_TESTNAME");
@@ -116,9 +117,9 @@ printFile(const char *file)
}
/* Now append the following line into the output file:
- * $file: $progname $testname */
+ * $file: $progname: $func: $testname */
-fprintf(fp, "%s: %s", file, progname);
+fprintf(fp, "%s: %s: %s", file, func, progname);
if (testname)
fprintf(fp, ": %s", testname);
@@ -128,8 +129,12 @@ printFile(const char *file)
fclose(fp);
}
+#define CHECK_PATH(path) \
+checkPath(path, __FUNCTION__)
+
static void
-checkPath(const char *path)
+checkPath(const char *path,
+ const char *func)
{
char *fullPath = NULL;
char *relPath = NULL;
@@ -160,7 +165,7 @@ checkPath(const char *path)
if (!STRPREFIX(path, abs_topsrcdir) &&
!STRPREFIX(path, abs_topbuilddir)) {
-printFile(path);
+printFile(path, func);
}
VIR_FREE(crippledPath);
@@ -180,7 +185,7 @@ int open(const char *path, int flags, ...)
init_syms();
-checkPath(path);
+CHECK_PATH(path);
if (flags & O_CREAT) {
va_list ap;
@@ -199,7 +204,7 @@ FILE *fopen(const char *path, const char *mode)
{
init_syms();
-checkPath(path);
+CHECK_PATH(path);
return real_fopen(path, mode);
}
@@ -209,7 +214,7 @@ int access(const char *path, int mode)
{
init_syms();
-checkPath(path);
+CHECK_PATH(path);
return real_access(path, mode);
}
@@ -239,7 +244,7 @@ int stat(const char *path, struct stat *sb)
{
init_syms();
-checkPath(path);
+checkPath(path, "stat");
return real_stat(path, sb);
}
@@ -250,7 +255,7 @@ int stat64(const char *path, struct stat64 *sb)
{
init_syms();
-checkPath(path);
+checkPath(path, "stat");
return real_stat64(path, sb);
}
@@ -262,7 +267,7 @@ __xstat(int ver, const char *path, struct stat *sb)
{
init_syms();
-checkPath(path);
+checkPath(path, "stat");
return real___xstat(ver, path, sb);
}
@@ -274,7 +279,7 @@ __xstat64(int ver, const char *path, struct stat64 *sb)
{
init_syms();
-checkPath(path);
+checkPath(path, "stat");
return real___xstat64(ver, path, sb);
}
@@ -286,7 +291,7 @@ lstat(const char *path, struct stat *sb)
{
init_syms();
-checkPath(path);
+checkPath(path, "lstat");
return real_lstat(path, sb);
}
@@ -298,7 +303,7 @@ lstat64(const char *path, struct stat64 *sb)
{
init_syms();
-checkPath(path);
+checkPath(path, "lstat");
return real_lstat64(path, sb);
}
@@ -310,7 +315,7 @@ __lxstat(int ver, const char *path, struct stat *sb)
{
init_syms();
-checkPath(path);
+checkPath(path, "lstat");
return real___lxstat(ver, path, sb);
}
@@ -322,7 +327,7 @@ __lxstat64(int ver, const char *path, struct stat64 *sb)
{
init_syms();
-checkPath(path);
+checkPath(path, "lstat");
return real___lxstat64(ver, path, sb);
}
@@ -337,7 +342,7 @@ int connect(int sockfd, const struct sockaddr *addr,
socklen_t addrlen)
if (addrlen == sizeof(struct sockaddr_un)) {
struct sockaddr_un *tmp = (struct sockaddr_un *) addr;
if (tmp->sun_family == AF_UNIX)
-checkPath(tmp->sun_path);
+CHECK_PATH(tmp->sun_path);
}
#endif
--
2.16.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 2/3] check-file-access: Allow specifying action
The check-file-access.pl script is used to match access list
generated by virtestmock against whitelisted rules stored in
file_access_whitelist.txt. So far the rules are in form:
$path: $progname: $testname
This is not sufficient because the rule does not take into
account 'action' that caused $path to appear in the list of
accessed files. After this commit the rule can be in new form:
$path: $action: $progname: $testname
where $action is one from ("open", "fopen", "access", "stat",
"lstat", "connect"). This way the white list can be fine tuned to
allow say access() but not connect().
Signed-off-by: Michal Privoznik
---
tests/check-file-access.pl | 32 +++-
tests/file_access_whitelist.txt | 15 ++-
2 files changed, 37 insertions(+), 10 deletions(-)
diff --git a/tests/check-file-access.pl b/tests/check-file-access.pl
index 977a2bc533..ea0b7a18a2 100755
--- a/tests/check-file-access.pl
+++ b/tests/check-file-access.pl
@@ -27,18 +27,21 @@ use warnings;
my $access_file = "test_file_access.txt";
my $whitelist_file = "file_access_whitelist.txt";
+my @known_actions = ("open", "fopen", "access", "stat", "lstat", "connect");
+
my @files;
my @whitelist;
open FILE, "<", $access_file or die "Unable to open $access_file: $!";
while () {
chomp;
-if (/^(\S*):\s*(\S*)(\s*:\s*(.*))?$/) {
+if (/^(\S*):\s*(\S*):\s*(\S*)(\s*:\s*(.*))?$/) {
my %rec;
${rec}{path} = $1;
-${rec}{progname} = $2;
-if (defined $4) {
-${rec}{testname} = $4;
+${rec}{action} = $2;
+${rec}{progname} = $3;
+if (defined $5) {
+${rec}{testname} = $5;
}
push (@files, \%rec);
} else {
@@ -52,7 +55,21 @@ while () {
chomp;
if (/^\s*#.*$/) {
# comment
+} elsif (/^(\S*):\s*(\S*)(:\s*(\S*)(\s*:\s*(.*))?)?$/ and
+grep /^$2$/, @known_actions) {
+# $path: $action: $progname: $testname
+my %rec;
+${rec}{path} = $1;
+${rec}{action} = $3;
+if (defined $4) {
+${rec}{progname} = $4;
+}
+if (defined $6) {
+${rec}{testname} = $6;
+}
+push (@whitelist, \%rec);
} elsif (/^(\S*)(:\s*(\S*)(\s*:\s*(.*))?)?$/) {
+# $path: $progname: $testname
my %rec;
${rec}{path} = $1;
if (defined $3) {
@@ -79,6 +96,11 @@ for my $file (@files) {
next;
}
+if (defined %${rule}{action} and
+not %${file}{action} =~ m/^$rule->{action}$/) {
+next;
+}
+
if (defined %${rule}{progname} and
not %${file}{progname} =~ m/^$rule->{progname}$/) {
next;
@@ -95,7 +117,7 @@ for my $file (@files) {
if (not $match) {
$error = 1;
-print "$file->{path}: $file->{progname}";
+print "$file->{path}: $file->{action}: $file->{progname}";
print ": $file->{testname}" if defined %${file}{testname};
print "\n";
}
diff --git a/tests/file_access_whitelist.txt b/tests/file_access_whitelist.txt
index 850b28506e..3fb318cbab 100644
--- a/tests/file_access_whitelist.txt
+++ b/tests/file_access_whitelist.txt
@@ -1,14 +1,17 @@
# This is a whitelist that allows accesses to files not in our
# build directory nor source directory. The records are in the
-# following format:
+# following formats:
#
# $path: $progname: $testname
+# $path: $action: $progname: $testname
#
-# All these three are evaluated as perl RE. So to allow /dev/sda
-# and /dev/sdb, you can just '/dev/sd[a-b]', or to allow
+# All these variables are evaluated as perl RE. So to allow
+# /dev/sda and /dev/sdb, you can just '/dev/sd[a-b]', or to allow
# /proc/$pid/status you can '/proc/\d+/status' and so on.
-# Moreover, $progname and $testname can be empty, in which which
-# case $path is allowed for all tests.
+# Moreover, $action, $progname and $testname can be empty, in which
+# which case $path is allowed for all tests. However, $action (if
+# specified) must be one of "open", "fopen", "access", "stat",
+# "lstat", "connect".
/bin/cat: sysinfotest
/bin/dirname: sysinfotest: x86 sysinfo
@@ -19,5 +22,7 @@
/etc/hosts
/proc/\d+/status
+/etc/passwd: fopen
+
# This is just a dummy example, DO NOT USE IT LIKE THAT!
.*: nonexistent-test-touching-everything
--
2.16.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 1/3] qemuxml2argvtest: Set more fake drivers
So far we are setting only fake secret and storage drivers. Therefore if the code wants to call a public NWFilter API (like qemuBuildInterfaceCommandLine() and qemuBuildNetCommandLine() are doing) the virGetConnectNWFilter() function will try to actually spawn session daemon because there's no connection object set to handle NWFilter driver. Even though I haven't experienced the same problem with the rest of the drivers (interface, network and node dev), the reasoning above can be applied to them as well. At the same time, now that connection object is registered for the drivers, the public APIs will throw virReportUnsupportedError(). And since we don't provide any error func the error is printed to stderr. Fix this by setting dummy error func. Signed-off-by: Michal Privoznik --- tests/qemuxml2argvtest.c | 4 1 file changed, 4 insertions(+) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 84117a3e63..8901c7bde4 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -471,6 +471,10 @@ testCompareXMLToArgv(const void *data) conn->secretDriver = conn->storageDriver = +virSetConnectInterface(conn); +virSetConnectNetwork(conn); +virSetConnectNWFilter(conn); +virSetConnectNodeDev(conn); virSetConnectSecret(conn); virSetConnectStorage(conn); -- 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 0/3] Don't touch user data from tests
v3 of: https://www.redhat.com/archives/libvir-list/2018-July/msg00747.html diff to v2: - pushed some already ACKed patches - dropped controversial line from 1/3 that turned off error reporting Michal Prívozník (3): qemuxml2argvtest: Set more fake drivers check-file-access: Allow specifying action virtestmock: Track action tests/check-file-access.pl | 32 +++- tests/file_access_whitelist.txt | 15 ++- tests/qemuxml2argvtest.c| 4 tests/virtestmock.c | 39 ++- 4 files changed, 63 insertions(+), 27 deletions(-) -- 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 1/3] qemuxml2argvtest: Set more fake drivers
On 07/27/2018 05:29 PM, Daniel P. Berrangé wrote: > On Fri, Jul 27, 2018 at 05:24:45PM +0200, Michal Privoznik wrote: >> So far we are setting only fake secret and storage drivers. >> Therefore if the code wants to call a public NWFilter API (like >> qemuBuildInterfaceCommandLine() and qemuBuildNetCommandLine() are >> doing) the virGetConnectNWFilter() function will try to actually >> spawn session daemon because there's no connection object set to >> handle NWFilter driver. >> >> Even though I haven't experienced the same problem with the rest >> of the drivers (interface, network and node dev), the reasoning >> above can be applied to them as well. >> >> At the same time, now that connection object is registered for >> the drivers, the public APIs will throw >> virReportUnsupportedError(). And since we don't provide any error >> func the error is printed to stderr. Fix this by setting dummy >> error func. > > Is this last paragraph stale ? you're not seeing a dummy error > func in this patch ... Oh yes, I am: libvirt.git/tests $ VIR_TEST_DEBUG=1 ./qemuxml2argvtest 2>&1 | grep "this function is not supported" 441) QEMU XML-2-ARGV net-vhostuser-multiq ... libvirt: Network Filter Driver error : this function is not supported by the connection driver: virNWFilterBindingLookupByPortDev libvirt: Network Filter Driver error : this function is not supported by the connection driver: virNWFilterBindingLookupByPortDev libvirt: Network Filter Driver error : this function is not supported by the connection driver: virNWFilterBindingLookupByPortDev 2018-07-27 16:02:50.689+: 11166: error : virNWFilterBindingLookupByPortDev:599 : this function is not supported by the connection driver: virNWFilterBindingLookupByPortDev 2018-07-27 16:02:50.689+: 11166: error : virNWFilterBindingLookupByPortDev:599 : this function is not supported by the connection driver: virNWFilterBindingLookupByPortDev 2018-07-27 16:02:50.689+: 11166: error : virNWFilterBindingLookupByPortDev:599 : this function is not supported by the connection driver: virNWFilterBindingLookupByPortDev (Or even without DEBUG) But as I told Peter, at this point stopping touching live user data is more important to me than silencing some error message. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] audit: Share virtType fallback logic
On 07/26/2018 01:59 PM, Cole Robinson wrote: > Signed-off-by: Cole Robinson > --- > src/conf/domain_audit.c | 91 + > 1 file changed, 28 insertions(+), 63 deletions(-) > Reviewed-by: John Ferlan John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Remove unnecessary virQEMUDriverPtr arguments
Signed-off-by Steven Albertson
---
src/qemu/qemu_block.c | 15 +++
src/qemu/qemu_block.h | 6 ++
2 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 66e6301210..8081bced9d 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -339,8 +339,7 @@ qemuBlockDiskDetectNodes(virDomainDiskDefPtr disk,
int
-qemuBlockNodeNamesDetect(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
+qemuBlockNodeNamesDetect(virDomainObjPtr vm,
qemuDomainAsyncJob asyncJob)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
@@ -354,13 +353,13 @@ qemuBlockNodeNamesDetect(virQEMUDriverPtr driver,
if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_NAMED_BLOCK_NODES))
return 0;
-if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+if (qemuDomainObjEnterMonitorAsync(priv->driver, vm, asyncJob) < 0)
return -1;
data = qemuMonitorQueryNamedBlockNodes(qemuDomainGetMonitor(vm));
blockstats = qemuMonitorQueryBlockstats(qemuDomainGetMonitor(vm), false);
-if (qemuDomainObjExitMonitor(driver, vm) < 0 || !data || !blockstats)
+if (qemuDomainObjExitMonitor(priv->driver, vm) < 0 || !data || !blockstats)
goto cleanup;
if (!(disktable = qemuBlockNodeNameGetBackingChain(data, blockstats)))
@@ -1689,14 +1688,14 @@ qemuBlockStorageSourceAttachRollback(qemuMonitorPtr mon,
* monitor internally.
*/
int
-qemuBlockStorageSourceDetachOneBlockdev(virQEMUDriverPtr driver,
-virDomainObjPtr vm,
+qemuBlockStorageSourceDetachOneBlockdev(virDomainObjPtr vm,
qemuDomainAsyncJob asyncJob,
virStorageSourcePtr src)
{
+qemuDomainObjPrivatePtr priv = vm->privateData;
int ret;
-if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+if (qemuDomainObjEnterMonitorAsync(priv->driver, vm, asyncJob) < 0)
return -1;
ret = qemuMonitorBlockdevDel(qemuDomainGetMonitor(vm), src->nodeformat);
@@ -1704,7 +1703,7 @@ qemuBlockStorageSourceDetachOneBlockdev(virQEMUDriverPtr
driver,
if (ret == 0)
ret = qemuMonitorBlockdevDel(qemuDomainGetMonitor(vm),
src->nodestorage);
-if (qemuDomainObjExitMonitor(driver, vm) < 0)
+if (qemuDomainObjExitMonitor(priv->driver, vm) < 0)
return -1;
return ret;
diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h
index fd8984e60b..725ce936d5 100644
--- a/src/qemu/qemu_block.h
+++ b/src/qemu/qemu_block.h
@@ -47,8 +47,7 @@ qemuBlockNodeNameGetBackingChain(virJSONValuePtr
namednodesdata,
virJSONValuePtr blockstats);
int
-qemuBlockNodeNamesDetect(virQEMUDriverPtr driver,
- virDomainObjPtr vm,
+qemuBlockNodeNamesDetect(virDomainObjPtr vm,
qemuDomainAsyncJob asyncJob);
virHashTablePtr
@@ -112,8 +111,7 @@ qemuBlockStorageSourceAttachRollback(qemuMonitorPtr mon,
qemuBlockStorageSourceAttachDataPtr data);
int
-qemuBlockStorageSourceDetachOneBlockdev(virQEMUDriverPtr driver,
-virDomainObjPtr vm,
+qemuBlockStorageSourceDetachOneBlockdev(virDomainObjPtr vm,
qemuDomainAsyncJob asyncJob,
virStorageSourcePtr src);
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] lxc: Don't return early in virLXCProcessSetupInterfaces
There are two places in the loop body that just return instead of
jumping onto the cleanup label. The problem is the cleanup code
is not ran in those cases.
Signed-off-by: Michal Privoznik
---
Pushed under trivial rule.
src/lxc/lxc_process.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index d021a890f7..442756a6f1 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -544,7 +544,7 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn,
net = def->nets[i];
if (virLXCProcessValidateInterface(net) < 0)
-return -1;
+goto cleanup;
if (virDomainNetAllocateActualDevice(def, net) < 0)
goto cleanup;
@@ -612,7 +612,7 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn,
/* Make sure all net definitions will have a name in the container */
if (!net->ifname_guest) {
if (virAsprintf(>ifname_guest, "eth%zu", niface) < 0)
-return -1;
+goto cleanup;
niface++;
}
}
--
2.16.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] lxc: Turn @veths into a string list in virLXCProcessStart
On 07/27/2018 10:37 AM, Erik Skultety wrote:
> On Thu, Jul 26, 2018 at 05:36:28PM +0200, Michal Privoznik wrote:
>> This way it will be easier to use autofree.
>>
>> Signed-off-by: Michal Privoznik
>> ---
>> src/lxc/lxc_process.c | 24 +---
>> 1 file changed, 9 insertions(+), 15 deletions(-)
>>
>> diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
>> index d021a890f7..3ac39d598c 100644
>> --- a/src/lxc/lxc_process.c
>> +++ b/src/lxc/lxc_process.c
>> @@ -514,8 +514,7 @@ static int virLXCProcessSetupNamespaces(virConnectPtr
>> conn,
>> * virLXCProcessSetupInterfaces:
>> * @conn: pointer to connection
>> * @def: pointer to virtual machine structure
>> - * @nveths: number of interfaces
>> - * @veths: interface names
>> + * @veths: string list of interface names
>> *
>> * Sets up the container interfaces by creating the veth device pairs and
>> * attaching the parent end to the appropriate bridge. The container end
>> @@ -525,7 +524,6 @@ static int virLXCProcessSetupNamespaces(virConnectPtr
>> conn,
>> */
>> static int virLXCProcessSetupInterfaces(virConnectPtr conn,
>> virDomainDefPtr def,
>> -size_t *nveths,
>> char ***veths)
>> {
>> int ret = -1;
>> @@ -534,6 +532,8 @@ static int virLXCProcessSetupInterfaces(virConnectPtr
>> conn,
>> virDomainNetDefPtr net;
>> virDomainNetType type;
>>
>> +*veths = NULL;
>> +
>> for (i = 0; i < def->nnets; i++) {
>> char *veth = NULL;
>> virNetDevBandwidthPtr actualBandwidth;
>> @@ -549,9 +549,6 @@ static int virLXCProcessSetupInterfaces(virConnectPtr
>> conn,
>> if (virDomainNetAllocateActualDevice(def, net) < 0)
>> goto cleanup;
>>
>> -if (VIR_EXPAND_N(*veths, *nveths, 1) < 0)
>> -goto cleanup;
>
> Contrary to what I said in the previous patch, I hadn't realized we were
> expanding a list by 1 in a loop before I looked at this patch. That is very
> inefficient and we'll keep doing that. Now I'm biased towards tracking the
> size
> of the list so that we can do VIR_EXPAND_N(*veths, 0, def->nnets) and then
> just
> add new elements, of course that would require tweaking the virStringListAdd
> more.
>
Hold on. I'm not quite sure I follow. Are you suggesting to pre-allocate
the string list and ditch virStringListAdd completely? Something like
this?
diff --git i/src/lxc/lxc_process.c w/src/lxc/lxc_process.c
index 6eef17d1ce..33c806630b 100644
--- i/src/lxc/lxc_process.c
+++ w/src/lxc/lxc_process.c
@@ -532,7 +532,8 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn,
virDomainNetDefPtr net;
virDomainNetType type;
-*veths = NULL;
+if (VIR_ALLOC_N(*veths, def->nnets + 1) < 0)
+return -1;
for (i = 0; i < def->nnets; i++) {
char *veth = NULL;
@@ -601,8 +602,7 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn,
}
}
-if (virStringListAdd(veths, veth) < 0)
-goto cleanup;
+(*veths)[i] = veth;
if (VIR_STRDUP(def->nets[i]->ifname_guest_actual, veth) < 0)
goto cleanup;
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH 2/4] tests: Move some test cases to test_nodedev
On Fri, Jul 27, 2018 at 02:37:39PM +0200, Andrea Bolognani wrote: > For whatever reason, a few nodedev-related test cases > have ended up in test_connect instead of the more > appropriate test_nodedev. Move them. > > Signed-off-by: Andrea Bolognani > --- https://www.redhat.com/mailman/listinfo/libvir-list Currently a test for some method is placed in the test file refering to the interface the tested method is exposed on. So NodeDeviceCreateXML method is exposed on the Connect Interface, this test_connect.py is the place for the test for this method. Katerina signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] util: clang is failing due to unused variables.
After some recent patches, clang is throwing some errors related to
unused variables. This is not happening when we use GCC with -Werror
enabled. Only clang reports this warning.
make[3]: Entering directory '/home/julio/Desktop/virt/libvirt/src'
CC util/libvirt_util_la-virscsivhost.lo
CC util/libvirt_util_la-virusb.lo
CC util/libvirt_util_la-virmdev.lo
util/virmdev.c:373:36: error: unused variable 'ret' [-Werror,-Wunused-variable]
VIR_AUTOPTR(virMediatedDevice) ret = virMediatedDeviceListSteal(list, dev);
^
1 error generated.
Makefile:11579: recipe for target 'util/libvirt_util_la-virmdev.lo' failed
make[3]: *** [util/libvirt_util_la-virmdev.lo] Error 1
make[3]: *** Waiting for unfinished jobs
util/virscsivhost.c:112:37: error: unused variable 'tmp'
[-Werror,-Wunused-variable]
VIR_AUTOPTR(virSCSIVHostDevice) tmp = virSCSIVHostDeviceListSteal(list,
dev);
^
1 error generated.
Makefile:11411: recipe for target 'util/libvirt_util_la-virscsivhost.lo' failed
make[3]: *** [util/libvirt_util_la-virscsivhost.lo] Error 1
util/virusb.c:511:31: error: unused variable 'ret' [-Werror,-Wunused-variable]
VIR_AUTOPTR(virUSBDevice) ret = virUSBDeviceListSteal(list, dev);
Signed-off-by: Julio Faracco
---
src/util/virmdev.c | 3 ++-
src/util/virscsivhost.c | 3 ++-
src/util/virusb.c | 3 ++-
3 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/src/util/virmdev.c b/src/util/virmdev.c
index 4050835cc1..63df4e40d8 100644
--- a/src/util/virmdev.c
+++ b/src/util/virmdev.c
@@ -370,7 +370,8 @@ void
virMediatedDeviceListDel(virMediatedDeviceListPtr list,
virMediatedDevicePtr dev)
{
-VIR_AUTOPTR(virMediatedDevice) ret = virMediatedDeviceListSteal(list, dev);
+VIR_AUTOPTR(virMediatedDevice) ret ATTRIBUTE_UNUSED
+= virMediatedDeviceListSteal(list, dev);
}
diff --git a/src/util/virscsivhost.c b/src/util/virscsivhost.c
index 280d0dc2fd..e07cd46ba9 100644
--- a/src/util/virscsivhost.c
+++ b/src/util/virscsivhost.c
@@ -109,7 +109,8 @@ void
virSCSIVHostDeviceListDel(virSCSIVHostDeviceListPtr list,
virSCSIVHostDevicePtr dev)
{
-VIR_AUTOPTR(virSCSIVHostDevice) tmp = virSCSIVHostDeviceListSteal(list,
dev);
+VIR_AUTOPTR(virSCSIVHostDevice) tmp ATTRIBUTE_UNUSED
+= virSCSIVHostDeviceListSteal(list, dev);
}
diff --git a/src/util/virusb.c b/src/util/virusb.c
index 609d54836f..73a1041097 100644
--- a/src/util/virusb.c
+++ b/src/util/virusb.c
@@ -508,7 +508,8 @@ void
virUSBDeviceListDel(virUSBDeviceListPtr list,
virUSBDevicePtr dev)
{
-VIR_AUTOPTR(virUSBDevice) ret = virUSBDeviceListSteal(list, dev);
+VIR_AUTOPTR(virUSBDevice) ret ATTRIBUTE_UNUSED
+= virUSBDeviceListSteal(list, dev);
}
virUSBDevicePtr
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: clang is failing due to unused variables.
On 07/27/2018 04:50 PM, Julio Faracco wrote:
> After some recent patches, clang is throwing some errors related to
> unused variables. This is not happening when we use GCC with -Werror
> enabled. Only clang reports this warning.
>
> make[3]: Entering directory '/home/julio/Desktop/virt/libvirt/src'
> CC util/libvirt_util_la-virscsivhost.lo
> CC util/libvirt_util_la-virusb.lo
> CC util/libvirt_util_la-virmdev.lo
> util/virmdev.c:373:36: error: unused variable 'ret'
> [-Werror,-Wunused-variable]
> VIR_AUTOPTR(virMediatedDevice) ret = virMediatedDeviceListSteal(list,
> dev);
>^
> 1 error generated.
> Makefile:11579: recipe for target 'util/libvirt_util_la-virmdev.lo' failed
> make[3]: *** [util/libvirt_util_la-virmdev.lo] Error 1
> make[3]: *** Waiting for unfinished jobs
> util/virscsivhost.c:112:37: error: unused variable 'tmp'
> [-Werror,-Wunused-variable]
> VIR_AUTOPTR(virSCSIVHostDevice) tmp = virSCSIVHostDeviceListSteal(list,
> dev);
> ^
> 1 error generated.
> Makefile:11411: recipe for target 'util/libvirt_util_la-virscsivhost.lo'
> failed
> make[3]: *** [util/libvirt_util_la-virscsivhost.lo] Error 1
> util/virusb.c:511:31: error: unused variable 'ret' [-Werror,-Wunused-variable]
> VIR_AUTOPTR(virUSBDevice) ret = virUSBDeviceListSteal(list, dev);
>
> Signed-off-by: Julio Faracco
> ---
> src/util/virmdev.c | 3 ++-
> src/util/virscsivhost.c | 3 ++-
> src/util/virusb.c | 3 ++-
> 3 files changed, 6 insertions(+), 3 deletions(-)
>
, see:
https://www.redhat.com/archives/libvir-list/2018-July/msg01917.html
Seems like it's the same thing and we should be consistent in the manner
in which we resolve.
John
> diff --git a/src/util/virmdev.c b/src/util/virmdev.c
> index 4050835cc1..63df4e40d8 100644
> --- a/src/util/virmdev.c
> +++ b/src/util/virmdev.c
> @@ -370,7 +370,8 @@ void
> virMediatedDeviceListDel(virMediatedDeviceListPtr list,
> virMediatedDevicePtr dev)
> {
> -VIR_AUTOPTR(virMediatedDevice) ret = virMediatedDeviceListSteal(list,
> dev);
> +VIR_AUTOPTR(virMediatedDevice) ret ATTRIBUTE_UNUSED
> += virMediatedDeviceListSteal(list, dev);
> }
>
>
> diff --git a/src/util/virscsivhost.c b/src/util/virscsivhost.c
> index 280d0dc2fd..e07cd46ba9 100644
> --- a/src/util/virscsivhost.c
> +++ b/src/util/virscsivhost.c
> @@ -109,7 +109,8 @@ void
> virSCSIVHostDeviceListDel(virSCSIVHostDeviceListPtr list,
>virSCSIVHostDevicePtr dev)
> {
> -VIR_AUTOPTR(virSCSIVHostDevice) tmp = virSCSIVHostDeviceListSteal(list,
> dev);
> +VIR_AUTOPTR(virSCSIVHostDevice) tmp ATTRIBUTE_UNUSED
> += virSCSIVHostDeviceListSteal(list, dev);
> }
>
>
> diff --git a/src/util/virusb.c b/src/util/virusb.c
> index 609d54836f..73a1041097 100644
> --- a/src/util/virusb.c
> +++ b/src/util/virusb.c
> @@ -508,7 +508,8 @@ void
> virUSBDeviceListDel(virUSBDeviceListPtr list,
> virUSBDevicePtr dev)
> {
> -VIR_AUTOPTR(virUSBDevice) ret = virUSBDeviceListSteal(list, dev);
> +VIR_AUTOPTR(virUSBDevice) ret ATTRIBUTE_UNUSED
> += virUSBDeviceListSteal(list, dev);
> }
>
> virUSBDevicePtr
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: clang is failing due to unused variables.
On 07/27/2018 03:58 PM, John Ferlan wrote: On 07/27/2018 04:50 PM, Julio Faracco wrote: After some recent patches, clang is throwing some errors related to unused variables. This is not happening when we use GCC with -Werror enabled. Only clang reports this warning. make[3]: Entering directory '/home/julio/Desktop/virt/libvirt/src' CC util/libvirt_util_la-virscsivhost.lo CC util/libvirt_util_la-virusb.lo CC util/libvirt_util_la-virmdev.lo util/virmdev.c:373:36: error: unused variable 'ret' [-Werror,-Wunused-variable] VIR_AUTOPTR(virMediatedDevice) ret = virMediatedDeviceListSteal(list, dev); ^ clang is buggy. The variable 'ret' is very much in use, as the VIR_AUTOPTR() macro cannot work unless you attach it to a local variable to operate on when that variable goes out of scope. You should file a bug report against clang. But in the meantime, , see: https://www.redhat.com/archives/libvir-list/2018-July/msg01917.html Seems like it's the same thing and we should be consistent in the manner in which we resolve. Yes, that approach is much nicer - it is also less typing and less magic. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: clang is failing due to unused variables.
Ow,
I was pretty sure that clang would complain about not handling the function
return properly. Well, I changed it and clang is not throwing any error. Weird!
Sending a V2.
Michael's patch was applied, so we need a new one to remove the other
occurrences.
Em sex, 27 de jul de 2018 às 17:59, John Ferlan escreveu:
>
>
>
> On 07/27/2018 04:50 PM, Julio Faracco wrote:
> > After some recent patches, clang is throwing some errors related to
> > unused variables. This is not happening when we use GCC with -Werror
> > enabled. Only clang reports this warning.
> >
> > make[3]: Entering directory '/home/julio/Desktop/virt/libvirt/src'
> > CC util/libvirt_util_la-virscsivhost.lo
> > CC util/libvirt_util_la-virusb.lo
> > CC util/libvirt_util_la-virmdev.lo
> > util/virmdev.c:373:36: error: unused variable 'ret'
> > [-Werror,-Wunused-variable]
> > VIR_AUTOPTR(virMediatedDevice) ret = virMediatedDeviceListSteal(list,
> > dev);
> >^
> > 1 error generated.
> > Makefile:11579: recipe for target 'util/libvirt_util_la-virmdev.lo' failed
> > make[3]: *** [util/libvirt_util_la-virmdev.lo] Error 1
> > make[3]: *** Waiting for unfinished jobs
> > util/virscsivhost.c:112:37: error: unused variable 'tmp'
> > [-Werror,-Wunused-variable]
> > VIR_AUTOPTR(virSCSIVHostDevice) tmp = virSCSIVHostDeviceListSteal(list,
> > dev);
> > ^
> > 1 error generated.
> > Makefile:11411: recipe for target 'util/libvirt_util_la-virscsivhost.lo'
> > failed
> > make[3]: *** [util/libvirt_util_la-virscsivhost.lo] Error 1
> > util/virusb.c:511:31: error: unused variable 'ret'
> > [-Werror,-Wunused-variable]
> > VIR_AUTOPTR(virUSBDevice) ret = virUSBDeviceListSteal(list, dev);
> >
> > Signed-off-by: Julio Faracco
> > ---
> > src/util/virmdev.c | 3 ++-
> > src/util/virscsivhost.c | 3 ++-
> > src/util/virusb.c | 3 ++-
> > 3 files changed, 6 insertions(+), 3 deletions(-)
> >
>
> , see:
>
> https://www.redhat.com/archives/libvir-list/2018-July/msg01917.html
>
> Seems like it's the same thing and we should be consistent in the manner
> in which we resolve.
>
> John
>
> > diff --git a/src/util/virmdev.c b/src/util/virmdev.c
> > index 4050835cc1..63df4e40d8 100644
> > --- a/src/util/virmdev.c
> > +++ b/src/util/virmdev.c
> > @@ -370,7 +370,8 @@ void
> > virMediatedDeviceListDel(virMediatedDeviceListPtr list,
> > virMediatedDevicePtr dev)
> > {
> > -VIR_AUTOPTR(virMediatedDevice) ret = virMediatedDeviceListSteal(list,
> > dev);
> > +VIR_AUTOPTR(virMediatedDevice) ret ATTRIBUTE_UNUSED
> > += virMediatedDeviceListSteal(list, dev);
> > }
> >
> >
> > diff --git a/src/util/virscsivhost.c b/src/util/virscsivhost.c
> > index 280d0dc2fd..e07cd46ba9 100644
> > --- a/src/util/virscsivhost.c
> > +++ b/src/util/virscsivhost.c
> > @@ -109,7 +109,8 @@ void
> > virSCSIVHostDeviceListDel(virSCSIVHostDeviceListPtr list,
> >virSCSIVHostDevicePtr dev)
> > {
> > -VIR_AUTOPTR(virSCSIVHostDevice) tmp =
> > virSCSIVHostDeviceListSteal(list, dev);
> > +VIR_AUTOPTR(virSCSIVHostDevice) tmp ATTRIBUTE_UNUSED
> > += virSCSIVHostDeviceListSteal(list, dev);
> > }
> >
> >
> > diff --git a/src/util/virusb.c b/src/util/virusb.c
> > index 609d54836f..73a1041097 100644
> > --- a/src/util/virusb.c
> > +++ b/src/util/virusb.c
> > @@ -508,7 +508,8 @@ void
> > virUSBDeviceListDel(virUSBDeviceListPtr list,
> > virUSBDevicePtr dev)
> > {
> > -VIR_AUTOPTR(virUSBDevice) ret = virUSBDeviceListSteal(list, dev);
> > +VIR_AUTOPTR(virUSBDevice) ret ATTRIBUTE_UNUSED
> > += virUSBDeviceListSteal(list, dev);
> > }
> >
> > virUSBDevicePtr
> >
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] util: clang is failing to compile due to unused variables.
After some recent patches, clang is throwing some errors related to
unused variables. This is not happening when we use GCC with -Werror
enabled. Only clang reports this warning.
make[3]: Entering directory '/home/julio/Desktop/virt/libvirt/src'
CC util/libvirt_util_la-virscsivhost.lo
CC util/libvirt_util_la-virusb.lo
CC util/libvirt_util_la-virmdev.lo
util/virmdev.c:373:36: error: unused variable 'ret' [-Werror,-Wunused-variable]
VIR_AUTOPTR(virMediatedDevice) ret = virMediatedDeviceListSteal(list, dev);
^
1 error generated.
Makefile:11579: recipe for target 'util/libvirt_util_la-virmdev.lo' failed
make[3]: *** [util/libvirt_util_la-virmdev.lo] Error 1
make[3]: *** Waiting for unfinished jobs
util/virscsivhost.c:112:37: error: unused variable 'tmp'
[-Werror,-Wunused-variable]
VIR_AUTOPTR(virSCSIVHostDevice) tmp = virSCSIVHostDeviceListSteal(list,
dev);
^
1 error generated.
Makefile:11411: recipe for target 'util/libvirt_util_la-virscsivhost.lo' failed
make[3]: *** [util/libvirt_util_la-virscsivhost.lo] Error 1
util/virusb.c:511:31: error: unused variable 'ret' [-Werror,-Wunused-variable]
VIR_AUTOPTR(virUSBDevice) ret = virUSBDeviceListSteal(list, dev);
Signed-off-by: Julio Faracco
---
src/util/virmdev.c | 2 +-
src/util/virscsivhost.c | 2 +-
src/util/virusb.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/util/virmdev.c b/src/util/virmdev.c
index 4050835cc1..4492fd673e 100644
--- a/src/util/virmdev.c
+++ b/src/util/virmdev.c
@@ -370,7 +370,7 @@ void
virMediatedDeviceListDel(virMediatedDeviceListPtr list,
virMediatedDevicePtr dev)
{
-VIR_AUTOPTR(virMediatedDevice) ret = virMediatedDeviceListSteal(list, dev);
+virMediatedDeviceListSteal(list, dev);
}
diff --git a/src/util/virscsivhost.c b/src/util/virscsivhost.c
index 280d0dc2fd..1a069e67ff 100644
--- a/src/util/virscsivhost.c
+++ b/src/util/virscsivhost.c
@@ -109,7 +109,7 @@ void
virSCSIVHostDeviceListDel(virSCSIVHostDeviceListPtr list,
virSCSIVHostDevicePtr dev)
{
-VIR_AUTOPTR(virSCSIVHostDevice) tmp = virSCSIVHostDeviceListSteal(list,
dev);
+virSCSIVHostDeviceListSteal(list, dev);
}
diff --git a/src/util/virusb.c b/src/util/virusb.c
index 609d54836f..d14b7623cc 100644
--- a/src/util/virusb.c
+++ b/src/util/virusb.c
@@ -508,7 +508,7 @@ void
virUSBDeviceListDel(virUSBDeviceListPtr list,
virUSBDevicePtr dev)
{
-VIR_AUTOPTR(virUSBDevice) ret = virUSBDeviceListSteal(list, dev);
+virUSBDeviceListSteal(list, dev);
}
virUSBDevicePtr
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
