Re: [libvirt] [PATCH v2 RESEND 00/12] PCI passthrough support on s390
在 2018/7/25 下午11:44, Andrea Bolognani 写道: On Wed, 2018-07-25 at 16:34 +0800, Yi Min Zhao wrote: 在 2018/7/24 下午11:43, Andrea Bolognani 写道: More concrete questions: one of the zPCI test cases includes which translates to -device zpci,uid=3,fid=2,target=pci.1,id=zpci3 \ -device pci-bridge,chassis_nr=1,id=pci.1,bus=pci.0,addr=0x1 \ -device zpci,uid=65535,fid=4294967295,target=hostdev0,id=zpci65535 \ -device vfio-pci,host=:00:00.0,id=hostdev0,bus=pci.1,addr=0x1f \ How does the pci-bridge controller show up in the guest, if at all? Qemu hides pci-bridge devices and just exposes pci devices to the guest. In above example, indeed, qemu will generate a pci-bridge device and it will be existing in pci topology. But the guest can't see it. This is very special. Yeah, that's kinda problematic as it violates the principle of least surprise... If s390 guests can only see a flat PCI topology, then we should find a way to reject bridges altogether instead of allowing the user to create them (or even create them automatically) only for them not to show up in the guest. If we reject bridges, there would be only one pci bus that maximum 32 pci devices could be plugged to. This kind of limitation is more problematic IMO. Do the bus= and addr= attributes of vfio-pci and pci-bridge in the example above matter eg. for migration purposes? Do you mean we leave attribute generation for bus and addr to qemu? That would be the idea, but of course it can only work if the address of the underlying PCI device can change without affecting the guest in any way, including migration. If that's not the case, and the PCI address needs to be as stable as the IDs, then I don't think there's a way to avoid storing it in the guest XML, no matter how confusing that will end up looking. I think it relies on pci base code. Although we don't expose pci address to the guest, any pci device still exists PCI tree tolopogy in qemu. IIUC, this has effect on qemu process itself. For example, if we hotplug a pci device permanently, it will be dynamically assigned with a pci address, and this address might change after shutdown and start again the guest and also might change in destination during migration. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/9] util: Add MBA capability information query to resctrl
On 2018年07月26日 06:31, John Ferlan wrote:
On 07/25/2018 12:28 AM, bing.niu wrote:
On 2018年07月25日 06:08, John Ferlan wrote:
[...]
+ * fatal in this case (errors out in next condition) - the
+ * kernel interface might've changed too much or something
else is
+ * wrong. */
"kernel" can fit on previous line meaning "wrong." can fit on previous
line. Also, no need for blank line between here and virReportError
Sorry I am a bit lost. Do you mean put "kernel" in previous line? right?
I have a try in Thunderbird. Looks like put “kernel” in previous line
beyond max length of line and Thunderbird give me automatically split. :(
Yes moving kernel up a line is what I meant. It's taken care of in my
branch already.
Thanks for this!
[...]
+ membw_info = resctrl->membw_info;
+ if (level > membw_info->last_level_cache) {
+ membw_info->last_level_cache = level;
+ membw_info->max_id = 0;
+ } else if (membw_info->last_level_cache == level) {
+ membw_info->max_id++;
+ }
+ }
+
This last hunk should be it's own patch. I can split it out for you.
The rest of the patch introduces the concept, does the query, and saves
the data in the "right place".
This hunk says, hey we have some membw_info data that can change our
perception of reality, so we need to adjust. Although nothing yet has
set last_level_cache or max_id... I'll assume that's comeing.
I'll also make membw_info "local" to the if {}.
The only hmm... I have is this last hunk, I've already forgotten what we
discussed the previous series. But my hmm is related to why is it here
and what impact can it (eventually) have if the values are changed in
this method while perhaps also being changed in a different method. I'm
sure I'll learn more of that as I move forward.
Thanks for that! Let me help to recall the discuss. :)
RDT kernel module will report some parameters for MBA. They are :
"min_bandwidth": The minimum memory bandwidth percentage which
user can request.
"bandwidth_gran": The granularity in which the memory bandwidth
percentage is allocated. The allocated
b/w percentage is rounded off to the next
control step available on the hardware. The
available bandwidth control steps are:
min_bandwidth + N * bandwidth_gran.
"num_closids": The number mba group can exist simultaneous.
Those information is query from kernel interface /sys/fs/resctrl/info/MB
Right this I understand the other fields are fetch-able. Setting
last_level_cache and max_id is only ever done in virResctrlInfoGetCache.
I have to remind myself what ends up calling into here and the order of
processing for all this code.
Above hunk is used to calculate the number of memory bandwidth
allocation controllers in system. The number of memory bandwidth
allocation controllers is same as last level cache. This number is
calculate by traversing cache hierarchy of
host(/sys/bus/cpu/devices/cpuX/cache/).
So above hunk is used to collect that information, to sanitize domain
XML about memory bandwidth allocation part. And the number of memory
bandwidth allocation controllers will not change, it just cannot query
directly from RDT kernel module.
So does the following seem like a good summary for the split out patch?:
util: Add MBA check to virResctrlInfoGetCache
If we have some membw_info data, then we need to calculate the number
of MBA controllers on the system. The value cannot be obtained from a
direct query to the RDT kernel module, but it is the same as the last
level cache value which is calculated by traversing the cache hierarchy
of host(/sys/bus/cpu/devices/cpuX/cache/).
Above summary is clear and informative. :)
John
Reviewed-by: John Ferlan
John
BTW: I'm stopping here for the evening, I'll work through the rest
hopefully tomorrow depending on interruptions.
Good evening :).
The rest are about 1. allocate memory bandwidth 2. domain XML and 3.
host capability XML.
if (level >= resctrl->nlevels)
return 0;
--
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] interactions between virDomainSetVcpusFlags and NUMA/pinning?
Hi, Just wondering about interactions between virDomainSetVcpusFlags() and virDomainPinVcpuFlags() and the domain XML. 1) If I add a vCPU to a domain, do I need to pin it after or does it respect the vCPU-to-pCPU mapping specified in the domain XML? 2) Are vCPUs added/removed in strict numerical order such that at any given time the active vCPUs are numbered 0-(N-1) where N is the number of active vCPUs? 3) Will the newly-added vCPUs respect the NUMA topology specified in the domain XML? Thanks, Chris -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/9] util: Add MBA allocation to virresctrl
On 07/18/2018 03:57 AM, bing@intel.com wrote:
> From: Bing Niu
>
> Add memory bandwidth allocation support to virresctrl class.
> Introducing virResctrlAllocMemBW which is used for allocating memory
> bandwidth. Following virResctrlAllocPerType, it also employs a
> nested sparse array to indicate whether allocation is available for
> particular last level cache.
>
> Signed-off-by: Bing Niu
> ---
> src/util/virresctrl.c | 346
> --
> src/util/virresctrl.h | 13 ++
> 2 files changed, 346 insertions(+), 13 deletions(-)
>
> diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c
> index 06e2702..bec2afd 100644
> --- a/src/util/virresctrl.c
> +++ b/src/util/virresctrl.c
> @@ -36,9 +36,9 @@ VIR_LOG_INIT("util.virresctrl")
>
>
> /* Resctrl is short for Resource Control. It might be implemented for
> various
> - * resources, but at the time of this writing this is only supported for
> cache
> - * allocation technology (aka CAT). Hence the reson for leaving 'Cache' out
> of
> - * all the structure and function names for now (can be added later if
> needed.
> + * resources. Currently this supports cache allocation technology (aka CAT)
> and
> + * memory bandwidth allocation (aka MBA). More resources technologies may be
> + * added in feature.
s/feature/the future/
> */
>
>
> @@ -89,6 +89,8 @@ typedef virResctrlAllocPerType *virResctrlAllocPerTypePtr;
> typedef struct _virResctrlAllocPerLevel virResctrlAllocPerLevel;
> typedef virResctrlAllocPerLevel *virResctrlAllocPerLevelPtr;
>
> +typedef struct _virResctrlAllocMemBW virResctrlAllocMemBW;
> +typedef virResctrlAllocMemBW *virResctrlAllocMemBWPtr;
>
> /* Class definitions and initializations */
> static virClassPtr virResctrlInfoClass;
> @@ -181,7 +183,10 @@ virResctrlInfoDispose(void *obj)
> * consequently a directory under /sys/fs/resctrl). Since it can have
> multiple
> * parts of multiple caches allocated it is represented as bunch of nested
> * sparse arrays (by sparse I mean array of pointers so that each might be
> NULL
> - * in case there is no allocation for that particular one (level, cache,
> ...)).
> + * in case there is no allocation for that particular cache allocation
> (level,
> + * cache, ...) or memory allocation for particular node).
> + *
> + * =Cache allocation technology (CAT)=
> *
> * Since one allocation can be made for caches on different levels, the first
> * nested sparse array is of types virResctrlAllocPerLevel. For example if
> you
> @@ -206,6 +211,16 @@ virResctrlInfoDispose(void *obj)
> * all of them. While doing that we store the bitmask in a sparse array of
> * virBitmaps named `masks` indexed the same way as `sizes`. The upper
> bounds
> * of the sparse arrays are stored in nmasks or nsizes, respectively.
> + *
> + * =Memory Bandwidth allocation technology (MBA)=
> + *
> + * The memory bandwidth allocation support in virResctrlAlloc works in the
> same
> + * fashion as CAT. However, memory bandwidth controller doesn't have a
> hierarchy
> + * organization as cache, each node have one memory bandwidth controller to
> + * memory bandwidth distribution. The number of memory bandwidth controller
> is
> + * identical with number of last level cache. So MBA also employs a sparse
> array
> + * to represent whether a memory bandwidth allocation happens on
> corresponding node.
> + * The available memory controller number is collected in 'virResctrlInfo'.
> */
> struct _virResctrlAllocPerType {
> /* There could be bool saying whether this is set or not, but since
> everything
> @@ -226,12 +241,24 @@ struct _virResctrlAllocPerLevel {
> * VIR_CACHE_TYPE_LAST number of items */
> };
>
> +/*
> + * virResctrlAllocMemBW represents one memory bandwidth allocation. Since it
> can have
> + * several last level caches in a NUMA system, it is also represented as a
> nested
> + * sparse arrays as virRestrlAllocPerLevel.
> + */
> +struct _virResctrlAllocMemBW {
> +unsigned int **bandwidths;
> +size_t nbandwidths;
> +};
> +
> struct _virResctrlAlloc {
> virObject parent;
>
> virResctrlAllocPerLevelPtr *levels;
> size_t nlevels;
>
> +virResctrlAllocMemBWPtr mem_bw;
> +
> /* The identifier (any unique string for now) */
> char *id;
> /* libvirt-generated path in /sys/fs/resctrl for this particular
> @@ -275,6 +302,13 @@ virResctrlAllocDispose(void *obj)
> VIR_FREE(level);
> }
>
> +if (alloc->mem_bw) {
> +virResctrlAllocMemBWPtr mem_bw = alloc->mem_bw;
> +for (i = 0; i < mem_bw->nbandwidths; i++)
> +VIR_FREE(mem_bw->bandwidths[i]);
> +}
> +
> +VIR_FREE(alloc->mem_bw);
NIT: Could be inside the if condition
> VIR_FREE(alloc->id);
> VIR_FREE(alloc->path);
> VIR_FREE(alloc->levels);
> @@ -697,6 +731,9 @@ virResctrlAllocIsEmpty(virResctrlAllocPtr alloc)
> if (!alloc)
>
Re: [libvirt] [PATCH 4/9] util: Add MBA capability information query to resctrl
On 07/25/2018 12:28 AM, bing.niu wrote:
>
>
> On 2018年07月25日 06:08, John Ferlan wrote:
[...]
>>
>>> + * fatal in this case (errors out in next condition) - the
>>> + * kernel interface might've changed too much or something
>>> else is
>>> + * wrong. */
>>
>> "kernel" can fit on previous line meaning "wrong." can fit on previous
>> line. Also, no need for blank line between here and virReportError
>
> Sorry I am a bit lost. Do you mean put "kernel" in previous line? right?
> I have a try in Thunderbird. Looks like put “kernel” in previous line
> beyond max length of line and Thunderbird give me automatically split. :(
Yes moving kernel up a line is what I meant. It's taken care of in my
branch already.
>>
[...]
>>
>>> + membw_info = resctrl->membw_info;
>>> + if (level > membw_info->last_level_cache) {
>>> + membw_info->last_level_cache = level;
>>> + membw_info->max_id = 0;
>>> + } else if (membw_info->last_level_cache == level) {
>>> + membw_info->max_id++;
>>> + }
>>> + }
>>> +
>>
>> This last hunk should be it's own patch. I can split it out for you.
>> The rest of the patch introduces the concept, does the query, and saves
>> the data in the "right place".
>>
>> This hunk says, hey we have some membw_info data that can change our
>> perception of reality, so we need to adjust. Although nothing yet has
>> set last_level_cache or max_id... I'll assume that's comeing.
>>
>> I'll also make membw_info "local" to the if {}.
>>
>> The only hmm... I have is this last hunk, I've already forgotten what we
>> discussed the previous series. But my hmm is related to why is it here
>> and what impact can it (eventually) have if the values are changed in
>> this method while perhaps also being changed in a different method. I'm
>> sure I'll learn more of that as I move forward.
>
> Thanks for that! Let me help to recall the discuss. :)
> RDT kernel module will report some parameters for MBA. They are :
> "min_bandwidth": The minimum memory bandwidth percentage which
> user can request.
>
> "bandwidth_gran": The granularity in which the memory bandwidth
> percentage is allocated. The allocated
> b/w percentage is rounded off to the next
> control step available on the hardware. The
> available bandwidth control steps are:
> min_bandwidth + N * bandwidth_gran.
>
> "num_closids": The number mba group can exist simultaneous.
>
> Those information is query from kernel interface /sys/fs/resctrl/info/MB
>
Right this I understand the other fields are fetch-able. Setting
last_level_cache and max_id is only ever done in virResctrlInfoGetCache.
I have to remind myself what ends up calling into here and the order of
processing for all this code.
> Above hunk is used to calculate the number of memory bandwidth
> allocation controllers in system. The number of memory bandwidth
> allocation controllers is same as last level cache. This number is
> calculate by traversing cache hierarchy of
> host(/sys/bus/cpu/devices/cpuX/cache/).
> So above hunk is used to collect that information, to sanitize domain
> XML about memory bandwidth allocation part. And the number of memory
> bandwidth allocation controllers will not change, it just cannot query
> directly from RDT kernel module.
So does the following seem like a good summary for the split out patch?:
util: Add MBA check to virResctrlInfoGetCache
If we have some membw_info data, then we need to calculate the number
of MBA controllers on the system. The value cannot be obtained from a
direct query to the RDT kernel module, but it is the same as the last
level cache value which is calculated by traversing the cache hierarchy
of host(/sys/bus/cpu/devices/cpuX/cache/).
John
>>
>> Reviewed-by: John Ferlan
>>
>> John
>>
>> BTW: I'm stopping here for the evening, I'll work through the rest
>> hopefully tomorrow depending on interruptions.
>
> Good evening :).
> The rest are about 1. allocate memory bandwidth 2. domain XML and 3.
> host capability XML.
>>
>>> if (level >= resctrl->nlevels)
>>> return 0;
>>>
>>
>> --
>> 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 v1 25/40] util: usb: use VIR_AUTOPTR for aggregate types
On Thu, 26 Jul 2018 at 00:32, Sukrit Bhatnagar wrote:
>
> On Wed, 25 Jul 2018 at 15:03, Erik Skultety wrote:
> >
> > On Sat, Jul 21, 2018 at 05:36:57PM +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 | 3 +--
> > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/src/util/virusb.c b/src/util/virusb.c
> > > index c14683f..cfeac51 100644
> > > --- a/src/util/virusb.c
> > > +++ b/src/util/virusb.c
> > > @@ -508,8 +508,7 @@ void
> > > virUSBDeviceListDel(virUSBDeviceListPtr list,
> > > virUSBDevicePtr dev)
> > > {
> > > -virUSBDevicePtr ret = virUSBDeviceListSteal(list, dev);
> > > -virUSBDeviceFree(ret);
> > > +VIR_AUTOPTR(virUSBDevice) ret = virUSBDeviceListSteal(list, dev);
> > > }
> > >
> >
> > Technically, there's also a virUSBDevicePtr instance in virUSBDeviceSearch
> > that
> > could be converted to VIR_AUTOPTR, but virUSBDeviceListAdd would have to
> > take a
> > double pointer to @dev instead of a single pointer. A bit more background
> > info - the current issue is that virUSBDeviceListAdd calls our
> > VIR_APPEND_ELEMENT helper which does clear the original pointer which we
> > could
> > utilize here, but not while passing a single pointer.
> > Not a deal breaker, though, it's just a nice to have, since you're already
> > working in this area, because I don't suppose we'd make such a change any
> > time
> > soon after your assignment is over.
> >
> > (regardless)
> > Reviewed-by: Erik Skultety
>
> There are many such functions:
> virMediatedDeviceListAdd
> virSCSIDeviceListAdd
> virPCIDeviceListAdd
>
> Making those changes would take a while and it is not directly related
> to our cleanup. So, I'll do the necessary after the two cleanup macros
> are used in all files. Is that ok?
For now, I am changing virUSBDeviceListAdd and the related occurrences.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 25/40] util: usb: use VIR_AUTOPTR for aggregate types
On Wed, 25 Jul 2018 at 15:03, Erik Skultety wrote:
>
> On Sat, Jul 21, 2018 at 05:36:57PM +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 | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/src/util/virusb.c b/src/util/virusb.c
> > index c14683f..cfeac51 100644
> > --- a/src/util/virusb.c
> > +++ b/src/util/virusb.c
> > @@ -508,8 +508,7 @@ void
> > virUSBDeviceListDel(virUSBDeviceListPtr list,
> > virUSBDevicePtr dev)
> > {
> > -virUSBDevicePtr ret = virUSBDeviceListSteal(list, dev);
> > -virUSBDeviceFree(ret);
> > +VIR_AUTOPTR(virUSBDevice) ret = virUSBDeviceListSteal(list, dev);
> > }
> >
>
> Technically, there's also a virUSBDevicePtr instance in virUSBDeviceSearch
> that
> could be converted to VIR_AUTOPTR, but virUSBDeviceListAdd would have to take
> a
> double pointer to @dev instead of a single pointer. A bit more background
> info - the current issue is that virUSBDeviceListAdd calls our
> VIR_APPEND_ELEMENT helper which does clear the original pointer which we could
> utilize here, but not while passing a single pointer.
> Not a deal breaker, though, it's just a nice to have, since you're already
> working in this area, because I don't suppose we'd make such a change any time
> soon after your assignment is over.
>
> (regardless)
> Reviewed-by: Erik Skultety
There are many such functions:
virMediatedDeviceListAdd
virSCSIDeviceListAdd
virPCIDeviceListAdd
Making those changes would take a while and it is not directly related
to our cleanup. So, I'll do the necessary after the two cleanup macros
are used in all files. Is that ok?
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 3/3] qemu: Use the correct vm def on cold attach
On 07/25/2018 09:51 AM, Ján Tomko wrote: > On Wed, Jul 25, 2018 at 08:39:26AM -0400, John Ferlan wrote: >> >> >> On 07/25/2018 06:40 AM, Michal Privoznik wrote: >>> On 07/16/2018 11:14 PM, John Ferlan wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1559867 When attaching a device to the domain we need to be sure to use the correct domain definition (vm->def or vm->newDef) when calling virDomainDeviceDefParse because the post parse processing algorithms that may assign an address for the device will use whatever domain definition was passed in. > > This effectively reduces AttachDevice(AFFECT_LIVE | AFFECT_CONFIG) to > two subsequent AttachDevice calls, thus possibly attaching two > different devices, making the AFFECT_CONFIG option pointless. > Ironically, the v2 response you gave was: " IMO this is a step into the right direction - rather than tuning up the device to be compatible with the live definition and hoping it will work in the persistent definition is just naive. " Still what I did doesn't change the AttachDevice calls. Prior to my changes and after my changes there is a qemuDomainAttachDeviceConfig for CONFIG and a qemuDomainAttachDeviceLive for LIVE. What the change does is separate the virDomainDeviceDefParse calls such that the "config" one is made with the persistent def rather than the live def for both. Thus if some previous change updates persistent, then the next change to just config doesn't miss that and perhaps duplicate something not provided - in this case the was duplicated which results in subsequent start failure. So looking at the old and new code side by side - how exactly is new code pointless? What subtle point am I missing? > E.g. when hotplugging a network interface, it might end up both on a > different PCI slot and with a different MAC address, which I'm afraid > might break some use cases. > An example would have greatly helped... So, I assume you mean this: # cat n1.xml # virsh attach-device $dom n1.xml --live --config Device attached successfully # virsh dumpxml $dom | grep interface -A 10 ... ... # virsh dumpxml $dom --inactive | grep interface -A 10 ... ... whereas, previously the same MAC address would have been generated for both live and config based on the current live def. Is that it? One other consideration. Prior to my changes. Assume the same n1.xml, then attach-device --config only, check out the resulting address using slot=0xd. Now use the --config --live - for the config guest you'll see that some MAC gets added at slot=0x0e while the same MAC gets added to slot=0x0d for the live guest. So I would contend this is no different than my changes other than with the changes the next guest wouldn't have the duplicated MAC from the live guest so the MAC being at a different address shouldn't matter. So the downside to libvirt generating a different MAC for live and config ends up being is what? The guest will boot the next time, but with a different MAC for the network device. If though a consumer doesn't provide one, how much of a problem is that compared to not being able to boot the guest because the conflict for disks. This is certainly one of those "competing interests" type problems for generated data. I really don't know the "perfect answer" other than telling customers to provide the specific things they want to avoid these conflicts. One could argue that if one wants a predictable MAC, then they should provide it. Similar argument for the . The one difference between the two for me being it's not possible to boot the machine with address conflicts; whereas, a change in an unprovided MAC is kind of a WYSIWYG type problem. Additionally, some devices (SCSI hostdev and SCSI disk) use algorithms that rely on knowing what already exists of the other type when generating the new device's address. Using the wrong VM definition could result in duplicated addresses. > > If suitablity for both live and persistent definition is a problem, > the address allocation code should take both domain definitions into > account and generate a single address for both device copies. > Could be a real challenge to do both - the algorithms aren't exactly "simple" now, especially when one has to take into account SCSI disks and SCSI hostdevs "share" the same bus. Not sure it'll work, but perhaps a different option - don't assign an address to a device that's being cold plugged while the domain is active. Although there's enough different types of addresses and devices that I'm not sure that's feasible for each type. In the case of the bz, two hostdev's with no domain address provided were added to the running domain's config only. However, the parsing algorithm used the live domain in order to figure out the host device address resulting in the same address
[libvirt] [PATCH RFC 36/39] qemu: driver: Allow using blockdev with qemuDomainBlocksStatsGather
Use the new monitor APIs to collect the data per blockdev when using the
new approach and add a new totalization function.
The data is reported for the 'format' layer.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_driver.c | 83 +++---
1 file changed, 59 insertions(+), 24 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ebcfdeb417..994625d305 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11030,14 +11030,10 @@ qemuDomainBlockResize(virDomainPtr dom,
}
-static int
-qemuDomainBlockStatsGatherTotals(void *payload,
- const void *name ATTRIBUTE_UNUSED,
- void *opaque)
+static void
+qemuDomainBlockStatsGatherTotalsData(qemuBlockStatsPtr data,
+ qemuBlockStatsPtr total)
{
-qemuBlockStatsPtr data = payload;
-qemuBlockStatsPtr total = opaque;
-
#define QEMU_BLOCK_STAT_TOTAL(NAME) \
if (data->NAME > 0) \
total->NAME += data->NAME
@@ -11051,6 +11047,30 @@ qemuDomainBlockStatsGatherTotals(void *payload,
QEMU_BLOCK_STAT_TOTAL(rd_total_times);
QEMU_BLOCK_STAT_TOTAL(flush_total_times);
#undef QEMU_BLOCK_STAT_TOTAL
+}
+
+
+static int
+qemuDomainBlockStatsGatherTotals(void *payload,
+ const void *name ATTRIBUTE_UNUSED,
+ void *opaque)
+{
+qemuDomainBlockStatsGatherTotalsData(payload, opaque);
+return 0;
+}
+
+
+static int
+qemuDomainBlockStatsGatherTotalsBlockdev(void *payload,
+ const void *name,
+ void *opaque)
+{
+const char *nodename = name;
+
+if (!strstr(nodename, "-format"))
+return 0;
+
+qemuDomainBlockStatsGatherTotalsData(payload, opaque);
return 0;
}
@@ -11079,7 +11099,8 @@ qemuDomainBlocksStatsGather(virQEMUDriverPtr driver,
virHashTablePtr blockstats = NULL;
qemuBlockStatsPtr stats;
int nstats;
-char *diskAlias = NULL;
+const char *entryname = NULL;
+bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV);
int ret = -1;
if (*path) {
@@ -11088,22 +11109,34 @@ qemuDomainBlocksStatsGather(virQEMUDriverPtr driver,
goto cleanup;
}
-if (!disk->info.alias) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("missing disk device alias name for %s"),
disk->dst);
-goto cleanup;
-}
+if (blockdev) {
+entryname = disk->src->nodeformat;
+} else {
+if (!disk->info.alias) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("missing disk device alias name for %s"),
disk->dst);
+goto cleanup;
+}
-if (VIR_STRDUP(diskAlias, disk->info.alias) < 0)
-goto cleanup;
+entryname = disk->info.alias;
+}
}
qemuDomainObjEnterMonitor(driver, vm);
-nstats = qemuMonitorGetAllBlockStatsInfo(priv->mon, , false);
+if (blockdev) {
+if (qemuMonitorGetBlockStatsInfo(priv->mon, , ) < 0)
+nstats = -1;
-if (capacity && nstats >= 0 &&
-qemuMonitorBlockStatsUpdateCapacity(priv->mon, blockstats, false) < 0)
-nstats = -1;
+if (capacity && nstats >= 0 &&
+qemuMonitorBlockStatsUpdateCapacityBlockdev(priv->mon, blockstats)
< 0)
+nstats = -1;
+} else {
+nstats = qemuMonitorGetAllBlockStatsInfo(priv->mon, ,
false);
+
+if (capacity && nstats >= 0 &&
+qemuMonitorBlockStatsUpdateCapacity(priv->mon, blockstats, false)
< 0)
+nstats = -1;
+}
if (qemuDomainObjExitMonitor(driver, vm) < 0 || nstats < 0)
goto cleanup;
@@ -1,22 +11144,24 @@ qemuDomainBlocksStatsGather(virQEMUDriverPtr driver,
if (VIR_ALLOC(*retstats) < 0)
goto cleanup;
-if (diskAlias) {
-if (!(stats = virHashLookup(blockstats, diskAlias))) {
+if (entryname) {
+if (!(stats = virHashLookup(blockstats, entryname))) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("cannot find statistics for device '%s'"),
diskAlias);
+ _("cannot find statistics for device '%s'"),
entryname);
goto cleanup;
}
**retstats = *stats;
} else {
-virHashForEach(blockstats, qemuDomainBlockStatsGatherTotals,
*retstats);
+if (blockdev)
+virHashForEach(blockstats,
qemuDomainBlockStatsGatherTotalsBlockdev, *retstats);
+else
+virHashForEach(blockstats, qemuDomainBlockStatsGatherTotals,
*retstats);
}
ret = nstats;
cleanup:
-VIR_FREE(diskAlias);
virHashFree(blockstats);
return ret;
}
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
[libvirt] [PATCH RFC 34/39] qemu: monitor: Add APIs for refreshing disk capacity when using -blockdev
Disk image size data are not contained in the reply of query-blockstats
but need to be gathered from query-block. For use with -blockdev we
really need to call 'query-named-block-nodes' and process it to retrieve
the correct data.
This patch introduces qemuMonitorBlockStatsUpdateCapacityBlockdev which
updates the capacity data by nodename rather than device name.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_monitor.c | 11 +++
src/qemu/qemu_monitor.h | 4
src/qemu/qemu_monitor_json.c | 46
src/qemu/qemu_monitor_json.h | 2 ++
4 files changed, 63 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index bc9c7d53b0..a69fbb0749 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2339,6 +2339,17 @@ qemuMonitorBlockStatsUpdateCapacity(qemuMonitorPtr mon,
}
+int
+qemuMonitorBlockStatsUpdateCapacityBlockdev(qemuMonitorPtr mon,
+virHashTablePtr stats)
+{
+VIR_DEBUG("stats=%p", stats);
+
+QEMU_CHECK_MONITOR(mon);
+
+return qemuMonitorJSONBlockStatsUpdateCapacityBlockdev(mon, stats);
+}
+
int
qemuMonitorBlockResize(qemuMonitorPtr mon,
const char *device,
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 8ede11c82c..d1a8f899d7 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -603,6 +603,10 @@ int qemuMonitorBlockStatsUpdateCapacity(qemuMonitorPtr mon,
bool backingChain)
ATTRIBUTE_NONNULL(2);
+int qemuMonitorBlockStatsUpdateCapacityBlockdev(qemuMonitorPtr mon,
+virHashTablePtr stats)
+ATTRIBUTE_NONNULL(2);
+
int qemuMonitorBlockResize(qemuMonitorPtr mon,
const char *dev_name,
unsigned long long size);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 02a3f09a8b..820e669d16 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2639,6 +2639,52 @@ qemuMonitorJSONBlockStatsUpdateCapacity(qemuMonitorPtr
mon,
}
+static int
+qemuMonitorJSONBlockStatsUpdateCapacityBlockdevWorker(size_t pos
ATTRIBUTE_UNUSED,
+ virJSONValuePtr val,
+ void *opaque)
+{
+virHashTablePtr stats = opaque;
+virJSONValuePtr image;
+const char *nodename;
+
+if (!(nodename = virJSONValueObjectGetString(val, "node-name")) ||
+!(image = virJSONValueObjectGetObject(val, "image"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("query-named-block-nodes entry was not in expected
format"));
+return -1;
+}
+
+if (qemuMonitorJSONBlockStatsUpdateCapacityData(image, nodename, stats) <
0)
+return -1;
+
+return 1; /* we don't want to steal the value from the JSON array */
+}
+
+
+int
+qemuMonitorJSONBlockStatsUpdateCapacityBlockdev(qemuMonitorPtr mon,
+virHashTablePtr stats)
+{
+virJSONValuePtr nodes;
+int ret = -1;
+
+if (!(nodes = qemuMonitorJSONQueryNamedBlockNodes(mon)))
+return -1;
+
+if (virJSONValueArrayForeachSteal(nodes,
+
qemuMonitorJSONBlockStatsUpdateCapacityBlockdevWorker,
+ stats) < 0)
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+virJSONValueFree(nodes);
+return ret;
+}
+
+
/* Return 0 on success, -1 on failure, or -2 if not supported. Size
* is in bytes. */
int qemuMonitorJSONBlockResize(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 6042f7161f..ebf4d4e78b 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -95,6 +95,8 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon,
int qemuMonitorJSONBlockStatsUpdateCapacity(qemuMonitorPtr mon,
virHashTablePtr stats,
bool backingChain);
+int qemuMonitorJSONBlockStatsUpdateCapacityBlockdev(qemuMonitorPtr mon,
+virHashTablePtr stats);
int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon,
virHashTablePtr hash,
int *nstats);
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 31/39] qemu: monitor: Prepare query-block calls for dropping of -drive
Calls to query block organize the data by using the 'device' field in
the returned JSON. When -blockdev is used instead of -drive the 'device'
field is empty and thus can't be used.
This patch allows callers to specify that the 'qdev' field shoudl be
usedinstead to refer to the devices by the QOM handle.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_driver.c | 2 +-
src/qemu/qemu_monitor.c | 5 +++--
src/qemu/qemu_monitor.h | 3 ++-
src/qemu/qemu_monitor_json.c | 33 +++--
src/qemu/qemu_monitor_json.h | 3 ++-
src/qemu/qemu_process.c | 2 +-
tests/qemumonitorjsontest.c | 2 +-
7 files changed, 37 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8fe51a0067..9114800821 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18749,7 +18749,7 @@ qemuDomainGetDiskErrors(virDomainPtr dom,
}
qemuDomainObjEnterMonitor(driver, vm);
-table = qemuMonitorGetBlockInfo(priv->mon);
+table = qemuMonitorGetBlockInfo(priv->mon, false);
if (qemuDomainObjExitMonitor(driver, vm) < 0)
goto endjob;
if (!table)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 3b88354f4f..9d58217376 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2219,7 +2219,8 @@ qemuDomainDiskInfoFree(void *value, const void *name
ATTRIBUTE_UNUSED)
virHashTablePtr
-qemuMonitorGetBlockInfo(qemuMonitorPtr mon)
+qemuMonitorGetBlockInfo(qemuMonitorPtr mon,
+bool blockdev)
{
int ret;
virHashTablePtr table;
@@ -2229,7 +2230,7 @@ qemuMonitorGetBlockInfo(qemuMonitorPtr mon)
if (!(table = virHashCreate(32, qemuDomainDiskInfoFree)))
return NULL;
-ret = qemuMonitorJSONGetBlockInfo(mon, table);
+ret = qemuMonitorJSONGetBlockInfo(mon, table, blockdev);
if (ret < 0) {
virHashFree(table);
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 26405ddd26..3e902e1e8b 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -561,7 +561,8 @@ int qemuMonitorSetMemoryStatsPeriod(qemuMonitorPtr mon,
int period);
int qemuMonitorBlockIOStatusToError(const char *status);
-virHashTablePtr qemuMonitorGetBlockInfo(qemuMonitorPtr mon);
+virHashTablePtr qemuMonitorGetBlockInfo(qemuMonitorPtr mon,
+bool blockdev);
virJSONValuePtr qemuMonitorQueryBlockstats(qemuMonitorPtr mon,
bool nodenames);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 8c1db3149e..6d76f4a2d5 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2187,11 +2187,12 @@ qemuMonitorJSONGetBlockDev(virJSONValuePtr devices,
static const char *
-qemuMonitorJSONGetBlockDevDevice(virJSONValuePtr dev)
+qemuMonitorJSONGetBlockDevName(virJSONValuePtr dev,
+ const char *key)
{
const char *thisdev;
-if (!(thisdev = virJSONValueObjectGetString(dev, "device"))) {
+if (!(thisdev = virJSONValueObjectGetString(dev, key))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("query-block device entry was not in expected
format"));
return NULL;
@@ -2201,8 +2202,23 @@ qemuMonitorJSONGetBlockDevDevice(virJSONValuePtr dev)
}
+static const char *
+qemuMonitorJSONGetBlockDevDevice(virJSONValuePtr dev)
+{
+return qemuMonitorJSONGetBlockDevName(dev, "device");
+}
+
+
+static const char *
+qemuMonitorJSONGetBlockDevQdev(virJSONValuePtr dev)
+{
+return qemuMonitorJSONGetBlockDevName(dev, "qdev");
+}
+
+
int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
-virHashTablePtr table)
+virHashTablePtr table,
+bool blockdev)
{
int ret = -1;
size_t i;
@@ -2223,10 +2239,15 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
if (!(dev = qemuMonitorJSONGetBlockDev(devices, i)))
goto cleanup;
-if (!(thisdev = qemuMonitorJSONGetBlockDevDevice(dev)))
-goto cleanup;
+if (blockdev) {
+if (!(thisdev = qemuMonitorJSONGetBlockDevQdev(dev)))
+goto cleanup;
+} else {
+if (!(thisdev = qemuMonitorJSONGetBlockDevDevice(dev)))
+goto cleanup;
-thisdev = qemuAliasDiskDriveSkipPrefix(thisdev);
+thisdev = qemuAliasDiskDriveSkipPrefix(thisdev);
+}
if (VIR_ALLOC(info) < 0)
goto cleanup;
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 3d1e40b53f..7cdec6e231 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -84,7 +84,8 @@ int qemuMonitorJSONSetMemoryStatsPeriod(qemuMonitorPtr mon,
char *balloonpath,
[libvirt] [PATCH RFC 30/39] qemu: hotplug: Implement removable media change for -blockdev
Use the new APIs which allow to manipulate the tray and media separately
and also allow using a nodename to refer to a media to implement media
changing.
With the new approach we don't have to call eject twice as the media is
removed by calling qemuMonitorBlockdevMediumRemove.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_hotplug.c | 93 -
1 file changed, 92 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 3eddb0043e..97a486f2ea 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -618,6 +618,93 @@ qemuHotplugDiskSourceRemove(qemuMonitorPtr mon,
}
+/**
+ * qemuDomainChangeMediaBlockdev:
+ * @driver: qemu driver structure
+ * @vm: domain definition
+ * @disk: disk definition to change the source of
+ * @newsrc: new disk source to change to
+ * @force: force the change of media
+ *
+ * Change the media in an ejectable device to the one described by
+ * @newsrc. This function also removes the old source from the
+ * shared device table if appropriate. Note that newsrc is consumed
+ * on success and the old source is freed on success.
+ *
+ * Returns 0 on success, -1 on error and reports libvirt error
+ */
+static int
+qemuDomainChangeMediaBlockdev(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainDiskDefPtr disk,
+ virStorageSourcePtr newsrc,
+ bool force)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+qemuHotplugDiskSourceDataPtr newbackend = NULL;
+qemuHotplugDiskSourceDataPtr oldbackend = NULL;
+virStorageSourcePtr oldsrc = disk->src;
+char *nodename = NULL;
+int rc;
+int ret = -1;
+
+if (!virStorageSourceIsEmpty(disk->src) &&
+!(oldbackend = qemuHotplugDiskSourceRemovePrepare(disk,
priv->qemuCaps)))
+goto cleanup;
+
+disk->src = newsrc;
+if (!virStorageSourceIsEmpty(disk->src)) {
+if (!(newbackend = qemuHotplugDiskSourceAttachPrepare(disk,
+ priv->qemuCaps)))
+goto cleanup;
+
+if (qemuDomainDiskGetBackendAlias(disk, priv->qemuCaps, ) < 0)
+goto cleanup;
+}
+
+if (diskPriv->tray && disk->tray_status != VIR_DOMAIN_DISK_TRAY_OPEN) {
+qemuDomainObjEnterMonitor(driver, vm);
+rc = qemuMonitorBlockdevTrayOpen(priv->mon, diskPriv->backendQomName,
force);
+if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
+goto cleanup;
+
+if (!force && qemuHotplugWaitForTrayEject(vm, disk) < 0)
+goto cleanup;
+}
+
+qemuDomainObjEnterMonitor(driver, vm);
+
+rc = qemuMonitorBlockdevMediumRemove(priv->mon, diskPriv->backendQomName);
+
+if (rc == 0 && oldbackend)
+qemuHotplugDiskSourceRemove(priv->mon, oldbackend);
+
+if (newbackend && nodename) {
+if (rc == 0)
+rc = qemuHotplugDiskSourceAttach(priv->mon, newbackend);
+
+if (rc == 0)
+rc = qemuMonitorBlockdevMediumInsert(priv->mon,
+ diskPriv->backendQomName,
+ nodename);
+}
+
+if (rc == 0)
+rc = qemuMonitorBlockdevTrayClose(priv->mon, diskPriv->backendQomName);
+
+if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0)
+goto cleanup;
+
+ cleanup:
+qemuHotplugDiskSourceDataFree(newbackend);
+qemuHotplugDiskSourceDataFree(oldbackend);
+/* caller handles correct exchange of sources */
+disk->src = oldsrc;
+return ret;
+}
+
+
/**
* qemuDomainChangeEjectableMedia:
* @driver: qemu driver structure
@@ -640,6 +727,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
virStorageSourcePtr newsrc,
bool force)
{
+qemuDomainObjPrivatePtr priv = vm->privateData;
int ret = -1;
int rc;
@@ -649,7 +737,10 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
if (qemuHotplugAttachManagedPR(driver, vm, newsrc, QEMU_ASYNC_JOB_NONE) <
0)
goto cleanup;
-rc = qemuDomainChangeMediaLegacy(driver, vm, disk, newsrc, force);
+if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV))
+rc = qemuDomainChangeMediaBlockdev(driver, vm, disk, newsrc, force);
+else
+rc = qemuDomainChangeMediaLegacy(driver, vm, disk, newsrc, force);
virDomainAuditDisk(vm, disk->src, newsrc, "update", rc >= 0);
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 29/39] qemu: monitor: Add APIs for cdrom tray handling for -blockdev
With blockdev we can use the full range of commands to manipulate the
tray and the medium separately. Implement monitor code for this.
Schema testing done in the qemumonitorjsontest allows us to verify that
we generate the commands correctly.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_monitor.c | 51 +++
src/qemu/qemu_monitor.h | 14 ++
src/qemu/qemu_monitor_json.c | 114 +++
src/qemu/qemu_monitor_json.h | 18 +++
tests/qemumonitorjsontest.c | 8 +++
5 files changed, 205 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index ac9cde4577..3b88354f4f 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4330,6 +4330,57 @@ qemuMonitorBlockdevDel(qemuMonitorPtr mon,
return qemuMonitorJSONBlockdevDel(mon, nodename);
}
+int
+qemuMonitorBlockdevTrayOpen(qemuMonitorPtr mon,
+const char *id,
+bool force)
+{
+VIR_DEBUG("id=%s force=%d", id, force);
+
+QEMU_CHECK_MONITOR(mon);
+
+return qemuMonitorJSONBlockdevTrayOpen(mon, id, force);
+}
+
+
+int
+qemuMonitorBlockdevTrayClose(qemuMonitorPtr mon,
+ const char *id)
+{
+VIR_DEBUG("id=%s", id);
+
+QEMU_CHECK_MONITOR(mon);
+
+return qemuMonitorJSONBlockdevTrayClose(mon, id);
+}
+
+
+int
+qemuMonitorBlockdevMediumRemove(qemuMonitorPtr mon,
+const char *id)
+{
+VIR_DEBUG("id=%s", id);
+
+QEMU_CHECK_MONITOR(mon);
+
+return qemuMonitorJSONBlockdevMediumRemove(mon, id);
+}
+
+
+
+int
+qemuMonitorBlockdevMediumInsert(qemuMonitorPtr mon,
+const char *id,
+const char *nodename)
+{
+VIR_DEBUG("id=%s nodename=%s", id, nodename);
+
+QEMU_CHECK_MONITOR(mon);
+
+return qemuMonitorJSONBlockdevMediumInsert(mon, id, nodename);
+}
+
+
char *
qemuMonitorGetSEVMeasurement(qemuMonitorPtr mon)
{
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index a25b1f54ea..26405ddd26 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1151,6 +1151,20 @@ int qemuMonitorBlockdevAdd(qemuMonitorPtr mon,
int qemuMonitorBlockdevDel(qemuMonitorPtr mon,
const char *nodename);
+int qemuMonitorBlockdevTrayOpen(qemuMonitorPtr mon,
+const char *id,
+bool force);
+
+int qemuMonitorBlockdevTrayClose(qemuMonitorPtr mon,
+ const char *id);
+
+int qemuMonitorBlockdevMediumRemove(qemuMonitorPtr mon,
+const char *id);
+
+int qemuMonitorBlockdevMediumInsert(qemuMonitorPtr mon,
+const char *id,
+const char *nodename);
+
char *
qemuMonitorGetSEVMeasurement(qemuMonitorPtr mon);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index f33535327c..8c1db3149e 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -8018,6 +8018,120 @@ qemuMonitorJSONBlockdevDel(qemuMonitorPtr mon,
return ret;
}
+
+int
+qemuMonitorJSONBlockdevTrayOpen(qemuMonitorPtr mon,
+const char *id,
+bool force)
+{
+virJSONValuePtr cmd;
+virJSONValuePtr reply = NULL;
+int ret = -1;
+
+if (!(cmd = qemuMonitorJSONMakeCommand("blockdev-open-tray",
+ "s:id", id,
+ "b:force", force, NULL)))
+return -1;
+
+if (qemuMonitorJSONCommand(mon, cmd, ) < 0)
+goto cleanup;
+
+if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
+
+
+int
+qemuMonitorJSONBlockdevTrayClose(qemuMonitorPtr mon,
+ const char *id)
+{
+virJSONValuePtr cmd;
+virJSONValuePtr reply = NULL;
+int ret = -1;
+
+if (!(cmd = qemuMonitorJSONMakeCommand("blockdev-close-tray",
+ "s:id", id, NULL)))
+return -1;
+
+if (qemuMonitorJSONCommand(mon, cmd, ) < 0)
+goto cleanup;
+
+if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
+
+
+int
+qemuMonitorJSONBlockdevMediumRemove(qemuMonitorPtr mon,
+const char *id)
+{
+virJSONValuePtr cmd;
+virJSONValuePtr reply = NULL;
+int ret = -1;
+
+if (!(cmd = qemuMonitorJSONMakeCommand("blockdev-remove-medium",
+ "s:id", id, NULL)))
+return -1;
+
+if (qemuMonitorJSONCommand(mon, cmd, ) < 0)
+goto cleanup;
+
+if (qemuMonitorJSONCheckError(cmd,
[libvirt] [PATCH RFC 39/39] DO NOT APPLY: Enable QEMU_CAPS_BLOCKDEV if 'copy-on-read' is supported
Use the 'copy-on-read' block drive in qemu as a witness to enable the
blockdev functionality and adapt the test files for the new output.
Note that the blockjobs were NOT adapted yet so any blockjob will desync
the state of qemu. This patch needs to wait until blockjobs are fixed.
---
src/qemu/qemu_capabilities.c | 1 +
tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml| 1 +
tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml | 1 +
tests/qemuxml2argvdata/disk-aio.x86_64-latest.args | 19 ++-
.../disk-backing-chains-noindex.x86_64-latest.args | 145 ++---
.../qemuxml2argvdata/disk-cache.x86_64-latest.args | 50 ---
.../disk-cdrom-network.x86_64-latest.args | 32 +++--
.../disk-cdrom-tray.x86_64-latest.args | 24 ++--
.../qemuxml2argvdata/disk-cdrom.x86_64-latest.args | 17 ++-
.../disk-copy_on_read.x86_64-latest.args | 19 ++-
.../disk-detect-zeroes.x86_64-latest.args | 17 ++-
.../disk-error-policy.x86_64-latest.args | 30 +++--
.../disk-network-gluster.x86_64-latest.args| 32 +++--
.../disk-network-iscsi.x86_64-latest.args | 58 +
.../disk-network-nbd.x86_64-latest.args| 41 --
.../disk-network-rbd.x86_64-latest.args| 67 ++
.../disk-network-sheepdog.x86_64-latest.args | 16 ++-
.../disk-network-source-auth.x86_64-latest.args| 30 +++--
.../disk-network-tlsx509.x86_64-latest.args| 61 +
.../disk-readonly-disk.x86_64-latest.args | 14 +-
.../disk-shared.x86_64-latest.args | 18 ++-
...isk-virtio-scsi-reservations.x86_64-latest.args | 20 ++-
22 files changed, 495 insertions(+), 218 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 6b4c14ac50..f439271ac6 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1309,6 +1309,7 @@ static struct virQEMUCapsStringFlags
virQEMUCapsQMPSchemaQueries[] = {
{ "blockdev-add/arg-type/+qcow2/encrypt/+luks/key-secret",
QEMU_CAPS_QCOW2_LUKS },
{ "nbd-server-start/arg-type/tls-creds", QEMU_CAPS_NBD_TLS },
{ "screendump/arg-type/device", QEMU_CAPS_SCREENDUMP_DEVICE },
+{ "blockdev-add/arg-type/+copy-on-read/file", QEMU_CAPS_BLOCKDEV },
};
typedef struct _virQEMUCapsObjectTypeProps virQEMUCapsObjectTypeProps;
diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml
b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml
index 01bb968938..01d9b684db 100644
--- a/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml
@@ -169,6 +169,7 @@
+
2012050
0
446771
diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml
index 4bc7cfeebc..f5d55dde4b 100644
--- a/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml
@@ -216,6 +216,7 @@
+
2012090
0
438109
diff --git a/tests/qemuxml2argvdata/disk-aio.x86_64-latest.args
b/tests/qemuxml2argvdata/disk-aio.x86_64-latest.args
index 3894ed2502..ae82ba52bd 100644
--- a/tests/qemuxml2argvdata/disk-aio.x86_64-latest.args
+++ b/tests/qemuxml2argvdata/disk-aio.x86_64-latest.args
@@ -24,13 +24,20 @@ file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
-no-acpi \
-boot strict=on \
-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
--drive file=/dev/HostVG/QEMUGuest1,format=qcow2,if=none,id=drive-ide0-0-0,\
-cache=none,aio=native \
--device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1,\
+-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1",\
+"aio":"native","node-name":"libvirt-2-storage","cache":{"direct":true,\
+"no-flush":false},"read-only":false,"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-2-format","read-only":false,\
+"cache":{"direct":true,"no-flush":false},"driver":"qcow2",\
+"file":"libvirt-2-storage"}' \
+-device
ide-hd,bus=ide.0,unit=0,drive=libvirt-2-format,id=ide0-0-0,bootindex=1,\
write-cache=on \
--drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-1-0,\
-readonly=on,aio=threads \
--device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \
+-blockdev '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest2",\
+"aio":"threads","node-name":"libvirt-1-storage","read-only":true,\
+"discard":"unmap"}' \
+-blockdev '{"node-name":"libvirt-1-format","read-only":true,"driver":"raw",\
+"file":"libvirt-1-storage"}' \
+-device ide-cd,bus=ide.1,unit=0,drive=libvirt-1-format,id=ide0-1-0 \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \
-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
resourcecontrol=deny \
diff --git
a/tests/qemuxml2argvdata/disk-backing-chains-noindex.x86_64-latest.args
b/tests/qemuxml2argvdata/disk-backing-chains-noindex.x86_64-latest.args
index afe078adcc..11d805f3d8 100644
---
[libvirt] [PATCH RFC 38/39] qemu: driver: Prepare qemuDomainGetStatsBlock (bulk disk stats) for -blockdev
Add code paths which call into the new functions to gather the data on a
per-node-name basis and tweak the aliases used for extracting the data.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_driver.c | 40
1 file changed, 28 insertions(+), 12 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 4f25471c4a..8755bd1652 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20206,6 +20206,7 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
virJSONValuePtr nodedata = NULL;
qemuDomainObjPrivatePtr priv = dom->privateData;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV);
bool fetchnodedata = virQEMUCapsGet(priv->qemuCaps,
QEMU_CAPS_QUERY_NAMED_BLOCK_NODES);
int count_index = -1;
@@ -20215,14 +20216,24 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
if (HAVE_JOB(privflags) && virDomainObjIsActive(dom)) {
qemuDomainObjEnterMonitor(driver, dom);
-rc = qemuMonitorGetAllBlockStatsInfo(priv->mon, ,
- visitBacking);
-if (rc >= 0)
-ignore_value(qemuMonitorBlockStatsUpdateCapacity(priv->mon, stats,
- visitBacking));
-if (fetchnodedata)
-nodedata = qemuMonitorQueryNamedBlockNodes(priv->mon);
+if (blockdev) {
+fetchnodedata = false;
+
+rc = qemuMonitorGetBlockStatsInfo(priv->mon, , NULL);
+
+if (rc >= 0)
+rc = qemuMonitorBlockStatsUpdateCapacityBlockdev(priv->mon,
stats);
+} else {
+rc = qemuMonitorGetAllBlockStatsInfo(priv->mon, ,
+ visitBacking);
+if (rc >= 0)
+ignore_value(qemuMonitorBlockStatsUpdateCapacity(priv->mon,
stats,
+
visitBacking));
+
+if (fetchnodedata)
+nodedata = qemuMonitorQueryNamedBlockNodes(priv->mon);
+}
if (qemuDomainObjExitMonitor(driver, dom) < 0)
goto cleanup;
@@ -20249,12 +20260,17 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
while (virStorageSourceIsBacking(src) &&
(src == disk->src || visitBacking)) {
-/* alias may be NULL if the VM is not running */
-if (disk->info.alias &&
-!(alias = qemuDomainStorageAlias(disk->info.alias, src->id)))
-goto cleanup;
+if (blockdev) {
+if (VIR_STRDUP(alias, src->nodeformat) < 0)
+goto cleanup;
+} else {
+/* alias may be NULL if the VM is not running */
+if (disk->info.alias &&
+!(alias = qemuDomainStorageAlias(disk->info.alias,
src->id)))
+goto cleanup;
-qemuDomainGetStatsOneBlockRefreshNamed(src, alias, stats,
nodestats);
+qemuDomainGetStatsOneBlockRefreshNamed(src, alias, stats,
nodestats);
+}
if (qemuDomainGetStatsOneBlock(driver, cfg, dom, record, maxparams,
disk->dst, alias, src, visited,
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 28/39] qemu: hotplug: Prepare for blockdev-add/blockdev-del with backing chains
Initialize data for the whole backing chain when plugging in or removing
disks when a machine supports -blockdev.
Similarly to startup we need to prepare the structures for the whole
backing chain and take care of the copy-on-read feature.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_hotplug.c | 75 -
1 file changed, 61 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 87efd3b411..3eddb0043e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -380,6 +380,10 @@ qemuHotplugRemoveManagedPR(virQEMUDriverPtr driver,
struct _qemuHotplugDiskSourceData {
qemuBlockStorageSourceAttachDataPtr *backends;
size_t nbackends;
+
+/* disk copy-on-read object */
+virJSONValuePtr corProps;
+char *corAlias;
};
typedef struct _qemuHotplugDiskSourceData qemuHotplugDiskSourceData;
typedef qemuHotplugDiskSourceData *qemuHotplugDiskSourceDataPtr;
@@ -393,6 +397,9 @@ qemuHotplugDiskSourceDataFree(qemuHotplugDiskSourceDataPtr
data)
if (!data)
return;
+virJSONValueFree(data->corProps);
+VIR_FREE(data->corAlias);
+
for (i = 0; i < data->nbackends; i++)
qemuBlockStorageSourceAttachDataFree(data->backends[i]);
@@ -461,25 +468,40 @@
qemuHotplugRemoveStorageSourcePrepareData(virStorageSourcePtr src,
static qemuHotplugDiskSourceDataPtr
qemuHotplugDiskSourceRemovePrepare(virDomainDiskDefPtr disk,
- virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED)
+ virQEMUCapsPtr qemuCaps)
{
+qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
qemuBlockStorageSourceAttachDataPtr backend = NULL;
qemuHotplugDiskSourceDataPtr data = NULL;
qemuHotplugDiskSourceDataPtr ret = NULL;
char *drivealias = NULL;
+virStorageSourcePtr n;
if (VIR_ALLOC(data) < 0)
return NULL;
-if (!(drivealias = qemuAliasDiskDriveFromDisk(disk)))
-goto cleanup;
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) {
+if (VIR_STRDUP(data->corAlias, diskPriv->nodeCopyOnRead) < 0)
+goto cleanup;
-if (!(backend = qemuHotplugRemoveStorageSourcePrepareData(disk->src,
- drivealias)))
-goto cleanup;
+for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore)
{
+if (!(backend = qemuHotplugRemoveStorageSourcePrepareData(n,
NULL)))
+goto cleanup;
-if (VIR_APPEND_ELEMENT(data->backends, data->nbackends, backend) < 0)
-goto cleanup;
+if (VIR_APPEND_ELEMENT(data->backends, data->nbackends, backend) <
0)
+goto cleanup;
+}
+} else {
+if (!(drivealias = qemuAliasDiskDriveFromDisk(disk)))
+goto cleanup;
+
+if (!(backend = qemuHotplugRemoveStorageSourcePrepareData(disk->src,
+ drivealias)))
+goto cleanup;
+
+if (VIR_APPEND_ELEMENT(data->backends, data->nbackends, backend) < 0)
+goto cleanup;
+}
VIR_STEAL_PTR(ret, data);
@@ -505,18 +527,36 @@ qemuHotplugDiskSourceAttachPrepare(virDomainDiskDefPtr
disk,
qemuBlockStorageSourceAttachDataPtr backend;
qemuHotplugDiskSourceDataPtr data;
qemuHotplugDiskSourceDataPtr ret = NULL;
+virStorageSourcePtr n;
if (VIR_ALLOC(data) < 0)
return NULL;
-if (!(backend = qemuBuildStorageSourceAttachPrepareDrive(disk, qemuCaps,
false)))
-goto cleanup;
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) {
+if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON &&
+!(data->corProps = qemuBlockStorageGetCopyOnReadProps(disk)))
+goto cleanup;
-if (qemuBuildStorageSourceAttachPrepareCommon(disk->src, backend,
qemuCaps) < 0)
-goto cleanup;
+for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore)
{
+if (!(backend = qemuBlockStorageSourceAttachPrepareBlockdev(n)))
+goto cleanup;
-if (VIR_APPEND_ELEMENT(data->backends, data->nbackends, backend) < 0)
-goto cleanup;
+if (qemuBuildStorageSourceAttachPrepareCommon(n, backend,
qemuCaps) < 0)
+goto cleanup;
+
+if (VIR_APPEND_ELEMENT(data->backends, data->nbackends, backend) <
0)
+goto cleanup;
+}
+} else {
+if (!(backend = qemuBuildStorageSourceAttachPrepareDrive(disk,
qemuCaps, false)))
+goto cleanup;
+
+if (qemuBuildStorageSourceAttachPrepareCommon(disk->src, backend,
qemuCaps) < 0)
+goto cleanup;
+
+if (VIR_APPEND_ELEMENT(data->backends, data->nbackends, backend) < 0)
+goto cleanup;
+}
VIR_STEAL_PTR(ret, data);
@@ -546,6 +586,10 @@ qemuHotplugDiskSourceAttach(qemuMonitorPtr mon,
return -1;
[libvirt] [PATCH RFC 37/39] qemu: monitor: Extract 'write-threshold' automatically for -blockdev
In cases when -blockdev is used we need to use 'query-named-block-nodes'
instead of 'query-block'. This means that we can extract the
write-threshold variable right away.
To keep compatibility with old VMs modify the code which was extracting
the value previously so that it updates the stats structure and a single
code path then can be used to extract the data.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_driver.c | 57 ++--
src/qemu/qemu_monitor.h | 3 +++
src/qemu/qemu_monitor_json.c | 16 ++---
3 files changed, 50 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 994625d305..4f25471c4a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20070,29 +20070,39 @@ qemuDomainGetStatsOneBlockFallback(virQEMUDriverPtr
driver,
}
-static int
-qemuDomainGetStatsOneBlockNode(virDomainStatsRecordPtr record,
- int *maxparams,
- virStorageSourcePtr src,
- size_t block_idx,
- virHashTablePtr nodedata)
+/**
+ * qemuDomainGetStatsOneBlockRefreshNamed:
+ * @src: disk source structure
+ * @alias: disk alias
+ * @stats: hash table containing stats for all disks
+ * @nodedata: reply containin 'query-named-block-nodes' data
+ *
+ * Refresh disk block stats data (qemuBlockStatsPtr) which are present only
+ * in the reply of 'query-named-block-nodes' in cases when the data was
gathered
+ * by using qem-block originally.
+ */
+static void
+qemuDomainGetStatsOneBlockRefreshNamed(virStorageSourcePtr src,
+ const char *alias,
+ virHashTablePtr stats,
+ virHashTablePtr nodedata)
{
+qemuBlockStatsPtr entry;
+
virJSONValuePtr data;
unsigned long long tmp;
-int ret = -1;
-if (src->nodestorage &&
-(data = virHashLookup(nodedata, src->nodestorage))) {
-if (virJSONValueObjectGetNumberUlong(data, "write_threshold", ) ==
0 &&
-tmp > 0)
-QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx,
- "threshold", tmp);
-}
+if (!nodedata || !src->nodestorage)
+return;
-ret = 0;
+if (!(entry = virHashLookup(stats, alias)))
+return;
- cleanup:
-return ret;
+if (!(data = virHashLookup(nodedata, src->nodestorage)))
+return;
+
+if (virJSONValueObjectGetNumberUlong(data, "write_threshold", ) == 0)
+entry->write_threshold = tmp;
}
@@ -20106,8 +20116,7 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver,
const char *entryname,
virStorageSourcePtr src,
size_t block_idx,
- virHashTablePtr stats,
- virHashTablePtr nodedata)
+ virHashTablePtr stats)
{
qemuBlockStats *entry;
int ret = -1;
@@ -20172,9 +20181,9 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver,
}
}
-if (qemuDomainGetStatsOneBlockNode(record, maxparams, src, block_idx,
- nodedata) < 0)
-goto cleanup;
+if (entry->write_threshold)
+QEMU_ADD_BLOCK_PARAM_ULL(record, maxparams, block_idx, "threshold",
+ entry->write_threshold);
ret = 0;
cleanup:
@@ -20245,9 +20254,11 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
!(alias = qemuDomainStorageAlias(disk->info.alias, src->id)))
goto cleanup;
+qemuDomainGetStatsOneBlockRefreshNamed(src, alias, stats,
nodestats);
+
if (qemuDomainGetStatsOneBlock(driver, cfg, dom, record, maxparams,
disk->dst, alias, src, visited,
- stats, nodestats) < 0)
+ stats) < 0)
goto cleanup;
VIR_FREE(alias);
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index d1a8f899d7..631ca732b3 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -585,6 +585,9 @@ struct _qemuBlockStats {
* if wr_highest_offset_valid is true */
unsigned long long wr_highest_offset;
bool wr_highest_offset_valid;
+
+/* write_threshold is valid only if it's non-zero, conforming to qemu
semantics */
+unsigned long long write_threshold;
};
int qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 820e669d16..3c67f5793b 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2539,7 +2539,8 @@ qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon,
static int
qemuMonitorJSONBlockStatsUpdateCapacityData(virJSONValuePtr image,
[libvirt] [PATCH RFC 35/39] qemu: driver: Don't pass 'virDomainDiskDefPtr' to qemuDomainGetStatsOneBlock
Allow reuse of qemuDomainGetStatsOneBlock to work with nodenames by
removing the code that looks up the stats data to the caller.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_driver.c | 24 +++-
1 file changed, 15 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9a3c4289ba..ebcfdeb417 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20067,7 +20067,8 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver,
virDomainObjPtr dom,
virDomainStatsRecordPtr record,
int *maxparams,
- virDomainDiskDefPtr disk,
+ const char *diskdst,
+ const char *entryname,
virStorageSourcePtr src,
size_t block_idx,
virHashTablePtr stats,
@@ -20075,13 +20076,9 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver,
{
qemuBlockStats *entry;
int ret = -1;
-char *alias = NULL;
-if (disk->info.alias)
-alias = qemuDomainStorageAlias(disk->info.alias, src->id);
+QEMU_ADD_NAME_PARAM(record, maxparams, "block", "name", block_idx,
diskdst);
-QEMU_ADD_NAME_PARAM(record, maxparams, "block", "name", block_idx,
-disk->dst);
if (virStorageSourceIsLocalStorage(src) && src->path)
QEMU_ADD_NAME_PARAM(record, maxparams, "block", "path",
block_idx, src->path);
@@ -20100,7 +20097,7 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver,
/* In case where qemu didn't provide the stats we stop here rather than
* trying to refresh the stats from the disk. Inability to provide stats is
* usually caused by blocked storage so this would make libvirtd hang */
-if (!stats || !alias || !(entry = virHashLookup(stats, alias))) {
+if (!stats || !entryname || !(entry = virHashLookup(stats, entryname))) {
ret = 0;
goto cleanup;
}
@@ -20146,7 +20143,6 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver,
ret = 0;
cleanup:
-VIR_FREE(alias);
return ret;
}
@@ -20171,6 +20167,7 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
int count_index = -1;
size_t visited = 0;
bool visitBacking = !!(privflags & QEMU_DOMAIN_STATS_BACKING);
+char *alias = NULL;
if (HAVE_JOB(privflags) && virDomainObjIsActive(dom)) {
qemuDomainObjEnterMonitor(driver, dom);
@@ -20207,10 +20204,18 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
while (virStorageSourceIsBacking(src) &&
(src == disk->src || visitBacking)) {
+
+/* alias may be NULL if the VM is not running */
+if (disk->info.alias &&
+!(alias = qemuDomainStorageAlias(disk->info.alias, src->id)))
+goto cleanup;
+
if (qemuDomainGetStatsOneBlock(driver, cfg, dom, record, maxparams,
- disk, src, visited,
+ disk->dst, alias, src, visited,
stats, nodestats) < 0)
goto cleanup;
+
+VIR_FREE(alias);
visited++;
src = src->backingStore;
}
@@ -20220,6 +20225,7 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
ret = 0;
cleanup:
+VIR_FREE(alias);
virHashFree(stats);
virHashFree(nodestats);
virJSONValueFree(nodedata);
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 26/39] qemu: process: Setup disk io throttling for -blockdev
The proper way to do this would be to use the 'throttle' driver but
unfortunately it can't change the 'throttle_group' so we can't provide
feature parity. This hack uses the block_set_io_throttle command to do
so until we can properly replace it.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_process.c | 50 +
1 file changed, 50 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 4b681a892e..928036172d 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6233,6 +6233,53 @@ qemuProcessGenID(virDomainObjPtr vm,
}
+/**
+ * qemuProcessSetupDiskThrottlingBlockdev:
+ *
+ * Sets up disk trottling for -blockdev via block_set_io_throttle monitor
+ * command. This hack should be replaced by proper use of the 'throttle'
+ * blockdev driver in qemu once it will support changing of the throttle group.
+ */
+static int
+qemuProcessSetupDiskThrottlingBlockdev(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ qemuDomainAsyncJob asyncJob)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+size_t i;
+int ret = -1;
+
+if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV))
+return 0;
+
+VIR_DEBUG("Setting up disk throttling for -blockdev via
block_set_io_throttle");
+
+if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
+return -1;
+
+for (i = 0; i < vm->def->ndisks; i++) {
+virDomainDiskDefPtr disk = vm->def->disks[i];
+qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+
+if (!qemuDiskConfigBlkdeviotuneEnabled(disk))
+continue;
+
+if (qemuMonitorSetBlockIoThrottle(qemuDomainGetMonitor(vm), NULL,
+ diskPriv->backendQomName,
+ >blkdeviotune,
+ true, true, true) < 0)
+goto cleanup;
+}
+
+ret = 0;
+
+ cleanup:
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ret = -1;
+return ret;
+}
+
+
/**
* qemuProcessLaunch:
*
@@ -6551,6 +6598,9 @@ qemuProcessLaunch(virConnectPtr conn,
if (qemuProcessSetupBalloon(driver, vm, asyncJob) < 0)
goto cleanup;
+if (qemuProcessSetupDiskThrottlingBlockdev(driver, vm, asyncJob) < 0)
+goto cleanup;
+
/* Since CPUs were not started yet, the balloon could not return the memory
* to the host and thus cur_balloon needs to be updated so that GetXMLdesc
* and friends return the correct size in case they can't grab the job */
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 12/39] qemu: process: Don't detect nodenames when we support -blockdev
We'll specify them ourselves so it's pointless to attempt to redetect them. Signed-off-by: Peter Krempa --- src/qemu/qemu_process.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 1d6cd72727..fbfba63fe1 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7805,7 +7805,8 @@ qemuProcessReconnect(void *opaque) if (qemuProcessRefreshDisks(driver, obj, QEMU_ASYNC_JOB_NONE) < 0) goto error; -if (qemuBlockNodeNamesDetect(driver, obj, QEMU_ASYNC_JOB_NONE) < 0) +if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV) && +qemuBlockNodeNamesDetect(driver, obj, QEMU_ASYNC_JOB_NONE) < 0) goto error; if (qemuRefreshVirtioChannelState(driver, obj, QEMU_ASYNC_JOB_NONE) < 0) -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 20/39] qemu: domain: Add field for storing node name for copy-on-read
The copy-on-read feature is expressed by adding a new node layer in
qemu when using -blockdev. Since we will keep these per-disk (as opposed
to per storage source) we need to store the appropriate node names in
the disk definition.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_domain.c| 11 +++
src/qemu/qemu_domain.h| 1 +
tests/qemustatusxml2xmldata/modern-in.xml | 3 +++
3 files changed, 15 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 9c1a2c6053..c98be208f1 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1066,6 +1066,7 @@ qemuDomainDiskPrivateDispose(void *obj)
VIR_FREE(priv->blockJobError);
virStorageSourceFree(priv->migrSource);
VIR_FREE(priv->backendQomName);
+VIR_FREE(priv->nodeCopyOnRead);
}
static virClassPtr qemuDomainStorageSourcePrivateClass;
@@ -2131,6 +2132,7 @@ qemuDomainDiskPrivateParse(xmlXPathContextPtr ctxt,
qemuDomainDiskPrivatePtr priv = QEMU_DOMAIN_DISK_PRIVATE(disk);
priv->backendQomName = virXPathString("string(./qom/@backend)", ctxt);
+priv->nodeCopyOnRead =
virXPathString("string(./nodenames/nodename[@type='copyOnRead']/@name)", ctxt);
return 0;
}
@@ -2144,6 +2146,15 @@ qemuDomainDiskPrivateFormat(virDomainDiskDefPtr disk,
virBufferEscapeString(buf, "\n", priv->backendQomName);
+if (priv->nodeCopyOnRead) {
+virBufferAddLit(buf, "\n");
+virBufferAdjustIndent(buf, 2);
+virBufferEscapeString(buf, "\n",
+ priv->nodeCopyOnRead);
+virBufferAdjustIndent(buf, -2);
+virBufferAddLit(buf, "\n");
+}
+
return 0;
}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 20208f56f5..758ffc0bb9 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -398,6 +398,7 @@ struct _qemuDomainDiskPrivate {
bool removable; /* device media can be removed/changed */
char *backendQomName; /* QOM path to the eligible block backend */
+char *nodeCopyOnRead; /* nodename of the disk-wide copy-on-read blockdev
layer */
};
# define QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src) \
diff --git a/tests/qemustatusxml2xmldata/modern-in.xml
b/tests/qemustatusxml2xmldata/modern-in.xml
index 21d4faca66..612090786a 100644
--- a/tests/qemustatusxml2xmldata/modern-in.xml
+++ b/tests/qemustatusxml2xmldata/modern-in.xml
@@ -333,6 +333,9 @@
+
+
+
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 27/39] qemu: driver: Use QOM backend name for disk IO throttling APIs
With -blockdev the drive alias can't be used any more so we need to
switch to the QOM name.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_driver.c | 30 --
1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0e1047c6ec..8fe51a0067 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18222,7 +18222,8 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
virDomainDefPtr def = NULL;
virDomainDefPtr persistentDef = NULL;
virDomainBlockIoTuneInfo info;
-char *device = NULL;
+char *drivealias = NULL;
+const char *qdevid = NULL;
int ret = -1;
size_t i;
virDomainDiskDefPtr conf_disk = NULL;
@@ -18447,8 +18448,12 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
if (!(disk = qemuDomainDiskByName(def, path)))
goto endjob;
-if (!(device = qemuAliasDiskDriveFromDisk(disk)))
-goto endjob;
+if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) {
+qdevid = QEMU_DOMAIN_DISK_PRIVATE(disk)->backendQomName;
+} else {
+if (!(drivealias = qemuAliasDiskDriveFromDisk(disk)))
+goto endjob;
+}
if (qemuDomainSetBlockIoTuneDefaults(, >blkdeviotune,
set_fields) < 0)
@@ -18494,7 +18499,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
* via the JSON error code from the block_set_io_throttle call */
qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, NULL,
+ret = qemuMonitorSetBlockIoThrottle(priv->mon, drivealias, qdevid,
, supportMaxOptions,
set_fields &
QEMU_BLOCK_IOTUNE_SET_GROUP_NAME,
supportMaxLengthOptions);
@@ -18544,7 +18549,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
cleanup:
VIR_FREE(info.group_name);
-VIR_FREE(device);
+VIR_FREE(drivealias);
virDomainObjEndAPI();
if (eventNparams)
virTypedParamsFree(eventParams, eventNparams);
@@ -18566,7 +18571,8 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
virDomainDefPtr def = NULL;
virDomainDefPtr persistentDef = NULL;
virDomainBlockIoTuneInfo reply = {0};
-char *device = NULL;
+char *drivealias = NULL;
+const char *qdevid = NULL;
int ret = -1;
int maxparams;
@@ -18620,10 +18626,14 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
if (!(disk = qemuDomainDiskByName(def, path)))
goto endjob;
-if (!(device = qemuAliasDiskDriveFromDisk(disk)))
-goto endjob;
+if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) {
+qdevid = QEMU_DOMAIN_DISK_PRIVATE(disk)->backendQomName;
+} else {
+if (!(drivealias = qemuAliasDiskDriveFromDisk(disk)))
+goto endjob;
+}
qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, NULL, );
+ret = qemuMonitorGetBlockIoThrottle(priv->mon, drivealias, qdevid,
);
if (qemuDomainObjExitMonitor(driver, vm) < 0)
goto endjob;
if (ret < 0)
@@ -18698,7 +18708,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
cleanup:
VIR_FREE(reply.group_name);
-VIR_FREE(device);
+VIR_FREE(drivealias);
virDomainObjEndAPI();
return ret;
}
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 25/39] qemu: command: Add helper to check if disk throttling is enabled
Add a helper which will use a collection of other helpers to determine
whether a disk requires throttling to be enabled.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_command.c | 10 ++
src/qemu/qemu_command.h | 3 +++
2 files changed, 13 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 7fa366cd3f..bfd1b99351 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1031,6 +1031,16 @@
qemuDiskConfigBlkdeviotuneHasMaxLength(virDomainDiskDefPtr disk)
}
+bool
+qemuDiskConfigBlkdeviotuneEnabled(virDomainDiskDefPtr disk)
+{
+return !!disk->blkdeviotune.group_name ||
+ qemuDiskConfigBlkdeviotuneHasBasic(disk) ||
+ qemuDiskConfigBlkdeviotuneHasMax(disk) ||
+ qemuDiskConfigBlkdeviotuneHasMaxLength(disk);
+}
+
+
/**
* qemuCheckDiskConfigBlkdeviotune:
* @disk: disk configuration
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index cf17dc1ede..b8f92a 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -181,6 +181,9 @@ int qemuGetDriveSourceString(virStorageSourcePtr src,
qemuDomainSecretInfoPtr secinfo,
char **source);
+bool
+qemuDiskConfigBlkdeviotuneEnabled(virDomainDiskDefPtr disk);
+
int qemuCheckDiskConfig(virDomainDiskDefPtr disk,
virQEMUCapsPtr qemuCaps);
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 18/39] qemu: Add field to store QDEV path of a disk in private data
When using -blockdev you need to use the qdev path to refer to the disk
fronends. Add means for storing the path and getting it after restart.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_domain.c| 27 +++
src/qemu/qemu_domain.h| 2 ++
tests/qemustatusxml2xmldata/modern-in.xml | 3 +++
3 files changed, 32 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 6103b86478..9c1a2c6053 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1065,6 +1065,7 @@ qemuDomainDiskPrivateDispose(void *obj)
VIR_FREE(priv->blockJobError);
virStorageSourceFree(priv->migrSource);
+VIR_FREE(priv->backendQomName);
}
static virClassPtr qemuDomainStorageSourcePrivateClass;
@@ -2123,6 +2124,30 @@ qemuStorageSourcePrivateDataFormat(virStorageSourcePtr
src,
}
+static int
+qemuDomainDiskPrivateParse(xmlXPathContextPtr ctxt,
+ virDomainDiskDefPtr disk)
+{
+qemuDomainDiskPrivatePtr priv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+
+priv->backendQomName = virXPathString("string(./qom/@backend)", ctxt);
+
+return 0;
+}
+
+
+static int
+qemuDomainDiskPrivateFormat(virDomainDiskDefPtr disk,
+virBufferPtr buf)
+{
+qemuDomainDiskPrivatePtr priv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+
+virBufferEscapeString(buf, "\n", priv->backendQomName);
+
+return 0;
+}
+
+
static void
qemuDomainObjPrivateXMLFormatVcpus(virBufferPtr buf,
virDomainDefPtr def)
@@ -2973,6 +2998,8 @@ virDomainXMLPrivateDataCallbacks
virQEMUDriverPrivateDataCallbacks = {
.alloc = qemuDomainObjPrivateAlloc,
.free = qemuDomainObjPrivateFree,
.diskNew = qemuDomainDiskPrivateNew,
+.diskParse = qemuDomainDiskPrivateParse,
+.diskFormat = qemuDomainDiskPrivateFormat,
.vcpuNew = qemuDomainVcpuPrivateNew,
.chrSourceNew = qemuDomainChrSourcePrivateNew,
.vsockNew = qemuDomainVsockPrivateNew,
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 57c130c047..20208f56f5 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -396,6 +396,8 @@ struct _qemuDomainDiskPrivate {
/* information about the device */
bool tray; /* device has tray */
bool removable; /* device media can be removed/changed */
+
+char *backendQomName; /* QOM path to the eligible block backend */
};
# define QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src) \
diff --git a/tests/qemustatusxml2xmldata/modern-in.xml
b/tests/qemustatusxml2xmldata/modern-in.xml
index 4fb5f326c2..21d4faca66 100644
--- a/tests/qemustatusxml2xmldata/modern-in.xml
+++ b/tests/qemustatusxml2xmldata/modern-in.xml
@@ -331,6 +331,9 @@
+
+
+
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 16/39] conf: Allow formatting and parsing of 'index' for disk source image
Similarly to backing store indexes which will become stable eventually
we need also to be able to format and store in the status XML for later
use the index for the top level of the backing chain.
Add XML formatter, parser, schema and docs.
Signed-off-by: Peter Krempa
---
docs/formatdomain.html.in | 7 ++-
docs/schemas/domaincommon.rng | 19 +++
src/conf/domain_conf.c | 21 +
.../qemuxml2argvdata/disk-backing-chains-index.xml | 12 ++--
.../disk-backing-chains-index-active.xml| 12 ++--
5 files changed, 54 insertions(+), 17 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 19b73125e1..45fcf18dcb 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2951,6 +2951,11 @@
is only valid when the specified storage volume is of 'file' or
'block' type).
+The source element may also have the index
+attribute with same semantics the
+index attribute of backingStore
+
+
The source element may contain the following sub elements:
@@ -3150,7 +3155,7 @@
by the backing store, see disk type attribute above for more
details and possible values.
- index
+ index
This attribute is only valid in output (and ignored on input) and
it can be used to refer to a specific part of the disk chain when
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index ac04af51a1..7c4e848685 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1528,6 +1528,14 @@
+
+
+
+
+
+
+
+
@@ -1551,6 +1559,7 @@
+
@@ -1575,6 +1584,7 @@
+
@@ -1600,6 +1610,7 @@
+
@@ -1653,6 +1664,7 @@
rbd
+
@@ -1692,6 +1704,7 @@
iscsi
+
@@ -1711,6 +1724,7 @@
+
@@ -1729,6 +1743,7 @@
+
@@ -1749,6 +1764,7 @@
+
@@ -1762,6 +1778,7 @@
gluster
+
@@ -1779,6 +1796,7 @@
+
@@ -1823,6 +1841,7 @@
+
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index cf52f9af5f..5c1842ebb6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9652,6 +9652,13 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
startupPolicy = virXMLPropString(cur, "startupPolicy");
+if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
+(tmp = virXMLPropString(cur, "index")) &&
+virStrToLong_uip(tmp, NULL, 10, >src->id) < 0) {
+virReportError(VIR_ERR_XML_ERROR, _("invalid disk index
'%s'"), tmp);
+goto error;
+}
+VIR_FREE(tmp);
} else if (!target &&
virXMLNodeNameEqual(cur, "target")) {
target = virXMLPropString(cur, "dev");
@@ -23648,6 +23655,7 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
int policy,
unsigned int flags,
bool skipSeclabels,
+ bool attrIndex,
virDomainXMLOptionPtr xmlopt)
{
virBuffer attrBuf = VIR_BUFFER_INITIALIZER;
@@ -23664,6 +23672,9 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
virBufferEscapeString(, " startupPolicy='%s'",
virDomainStartupPolicyTypeToString(policy));
+if (attrIndex && src->id != 0)
+virBufferAsprintf(, " index='%u'", src->id);
+
if (virDomainDiskSourceFormatPrivateData(, src, flags, xmlopt) <
0)
goto cleanup;
@@ -23686,7 +23697,8 @@ virDomainDiskSourceFormat(virBufferPtr buf,
unsigned int flags,
virDomainXMLOptionPtr xmlopt)
{
-return virDomainDiskSourceFormatInternal(buf, src, policy, flags, false,
xmlopt);
+return virDomainDiskSourceFormatInternal(buf, src, policy, flags, false,
+ false, xmlopt);
}
@@ -23728,7 +23740,8 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf,
virBufferAsprintf(buf, "\n", format);
/*
[libvirt] [PATCH RFC 33/39] qemu: monitor: Add API to retrieve blockstats by nodenames
Add workers that allow collecting the 'blockstats' from qemu by
nodename. This will replace the old usage when using -blockdev as
query-blockstats does not report anything when using the old approach.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_monitor.c | 25
src/qemu/qemu_monitor.h | 6
src/qemu/qemu_monitor_json.c | 69
src/qemu/qemu_monitor_json.h | 4 +++
4 files changed, 104 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 9d58217376..bc9c7d53b0 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2300,6 +2300,31 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon,
}
+int
+qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon,
+ virHashTablePtr *ret_stats,
+ int *nstats)
+{
+virHashTablePtr hash = NULL;
+int ret = -1;
+
+QEMU_CHECK_MONITOR(mon);
+
+if (!(hash = virHashCreate(10, virHashValueFree)))
+goto cleanup;
+
+if (qemuMonitorJSONGetBlockStatsInfo(mon, hash, nstats) < 0)
+goto cleanup;
+
+VIR_STEAL_PTR(*ret_stats, hash);
+ret = 0;
+
+ cleanup:
+virHashFree(hash);
+return ret;
+}
+
+
/* Updates "stats" to fill virtual and physical size of the image */
int
qemuMonitorBlockStatsUpdateCapacity(qemuMonitorPtr mon,
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 3e902e1e8b..8ede11c82c 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -592,6 +592,12 @@ int qemuMonitorGetAllBlockStatsInfo(qemuMonitorPtr mon,
bool backingChain)
ATTRIBUTE_NONNULL(2);
+int
+qemuMonitorGetBlockStatsInfo(qemuMonitorPtr mon,
+ virHashTablePtr *ret_stats,
+ int *nstats)
+ATTRIBUTE_NONNULL(2);
+
int qemuMonitorBlockStatsUpdateCapacity(qemuMonitorPtr mon,
virHashTablePtr stats,
bool backingChain)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 6d76f4a2d5..02a3f09a8b 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -2467,6 +2467,75 @@ qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon,
}
+struct qemuMonitorJSONGetBlockstatsInfoArrayWorkerData {
+virHashTablePtr hash;
+int nstats;
+};
+
+
+static int
+qemuMonitorJSONGetBlockStatsInfoArrayWorker(size_t pos ATTRIBUTE_UNUSED,
+virJSONValuePtr val,
+void *opaque)
+{
+struct qemuMonitorJSONGetBlockstatsInfoArrayWorkerData *data = opaque;
+qemuBlockStatsPtr bstats = NULL;
+const char *nodename;
+int nstats = 0;
+int ret = -1;
+
+if (!(nodename = virJSONValueObjectGetString(val, "node-name"))) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("missing node name in query-blockstats reply"));
+return -1;
+}
+
+if (!(bstats = qemuMonitorJSONBlockStatsCollectData(val, )))
+goto cleanup;
+
+if (nstats > data->nstats)
+data->nstats = nstats;
+
+if (virHashAddEntry(data->hash, nodename, bstats) < 0)
+goto cleanup;
+bstats = NULL;
+
+ret = 1; /* we don't want to steal the value from the JSON array */
+
+ cleanup:
+VIR_FREE(bstats);
+return ret;
+}
+
+
+int
+qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon,
+ virHashTablePtr hash,
+ int *nstats)
+{
+struct qemuMonitorJSONGetBlockstatsInfoArrayWorkerData data = { hash, 0 };
+virJSONValuePtr devices;
+int ret = -1;
+
+if (!(devices = qemuMonitorJSONQueryBlockstats(mon, true)))
+return -1;
+
+if (virJSONValueArrayForeachSteal(devices,
+
qemuMonitorJSONGetBlockStatsInfoArrayWorker,
+ ) < 0)
+goto cleanup;
+
+if (nstats)
+*nstats = data.nstats;
+
+ret = 0;
+
+ cleanup:
+virJSONValueFree(devices);
+return ret;
+}
+
+
static int
qemuMonitorJSONBlockStatsUpdateCapacityData(virJSONValuePtr image,
const char *name,
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 7cdec6e231..6042f7161f 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -95,6 +95,10 @@ int qemuMonitorJSONGetAllBlockStatsInfo(qemuMonitorPtr mon,
int qemuMonitorJSONBlockStatsUpdateCapacity(qemuMonitorPtr mon,
virHashTablePtr stats,
bool backingChain);
+int qemuMonitorJSONGetBlockStatsInfo(qemuMonitorPtr mon,
+ virHashTablePtr hash,
+ int *nstats);
+
int
[libvirt] [PATCH RFC 17/39] qemu: Use proper backingIndex when reporting stats for backing chain
Use the index stored in virStorageSource struct rather than
recalculating it. Currently we'd report proper numbers but that will
change with blockdev.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_driver.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b81ad7cdbc..0e1047c6ec 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20053,7 +20053,6 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver,
virDomainDiskDefPtr disk,
virStorageSourcePtr src,
size_t block_idx,
- unsigned int backing_idx,
virHashTablePtr stats,
virHashTablePtr nodedata)
{
@@ -20062,16 +20061,16 @@ qemuDomainGetStatsOneBlock(virQEMUDriverPtr driver,
char *alias = NULL;
if (disk->info.alias)
-alias = qemuDomainStorageAlias(disk->info.alias, backing_idx);
+alias = qemuDomainStorageAlias(disk->info.alias, src->id);
QEMU_ADD_NAME_PARAM(record, maxparams, "block", "name", block_idx,
disk->dst);
if (virStorageSourceIsLocalStorage(src) && src->path)
QEMU_ADD_NAME_PARAM(record, maxparams, "block", "path",
block_idx, src->path);
-if (backing_idx)
+if (src->id)
QEMU_ADD_BLOCK_PARAM_UI(record, maxparams, block_idx, "backingIndex",
-backing_idx);
+src->id);
/* the VM is offline so we have to go and load the stast from the disk by
* ourselves */
@@ -20188,16 +20187,14 @@ qemuDomainGetStatsBlock(virQEMUDriverPtr driver,
for (i = 0; i < dom->def->ndisks; i++) {
virDomainDiskDefPtr disk = dom->def->disks[i];
virStorageSourcePtr src = disk->src;
-unsigned int backing_idx = 0;
while (virStorageSourceIsBacking(src) &&
- (backing_idx == 0 || visitBacking)) {
+ (src == disk->src || visitBacking)) {
if (qemuDomainGetStatsOneBlock(driver, cfg, dom, record, maxparams,
- disk, src, visited, backing_idx,
+ disk, src, visited,
stats, nodestats) < 0)
goto cleanup;
visited++;
-backing_idx++;
src = src->backingStore;
}
}
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 23/39] qemu: domain: Prepare qemuDomainDiskGetBackendAlias for -blockdev
Pass in the node name as the backend alias when -blockdev is used. As
copy-on-read is expressed by a separate -blockdev backing chain member
we need to decide which node name to use here.
For empty cdroms when using -blockdev there is no backend at all so NULL
is returned.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_domain.c | 21 +++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 526634d819..ea3929ce7e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -8676,12 +8676,29 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
*/
int
qemuDomainDiskGetBackendAlias(virDomainDiskDefPtr disk,
- virQEMUCapsPtr qemuCaps ATTRIBUTE_UNUSED,
+ virQEMUCapsPtr qemuCaps,
char **backendAlias)
{
+qemuDomainDiskPrivatePtr priv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+const char *nodename = NULL;
*backendAlias = NULL;
-if (!(*backendAlias = qemuAliasDiskDriveFromDisk(disk)))
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) {
+if (!(*backendAlias = qemuAliasDiskDriveFromDisk(disk)))
+return -1;
+
+return 0;
+}
+
+if (virStorageSourceIsEmpty(disk->src))
+return 0;
+
+if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON)
+nodename = priv->nodeCopyOnRead;
+else
+nodename = disk->src->nodeformat;
+
+if (VIR_STRDUP(*backendAlias, nodename) < 0)
return -1;
return 0;
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 32/39] qemu: Use QOM path with query-block when using -blockdev
Switch to using the QOM/qdev handles in all calls to
qemuMonitorGetBlockInfo when using -blockdev. The callers also need to
make sure to use the correct handle afterwards to extract the data.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_driver.c | 11 +--
src/qemu/qemu_process.c | 9 +++--
2 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9114800821..9a3c4289ba 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18723,6 +18723,7 @@ qemuDomainGetDiskErrors(virDomainPtr dom,
virDomainObjPtr vm = NULL;
qemuDomainObjPrivatePtr priv;
virHashTablePtr table = NULL;
+bool blockdev = false;
int ret = -1;
size_t i;
int n = 0;
@@ -18733,6 +18734,7 @@ qemuDomainGetDiskErrors(virDomainPtr dom,
goto cleanup;
priv = vm->privateData;
+blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV);
if (virDomainGetDiskErrorsEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
@@ -18749,7 +18751,7 @@ qemuDomainGetDiskErrors(virDomainPtr dom,
}
qemuDomainObjEnterMonitor(driver, vm);
-table = qemuMonitorGetBlockInfo(priv->mon, false);
+table = qemuMonitorGetBlockInfo(priv->mon, blockdev);
if (qemuDomainObjExitMonitor(driver, vm) < 0)
goto endjob;
if (!table)
@@ -18758,8 +18760,13 @@ qemuDomainGetDiskErrors(virDomainPtr dom,
for (i = n = 0; i < vm->def->ndisks; i++) {
struct qemuDomainDiskInfo *info;
virDomainDiskDefPtr disk = vm->def->disks[i];
+qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+const char *entryname = disk->info.alias;
+
+if (blockdev)
+entryname = diskPriv->backendQomName;
-if ((info = virHashLookup(table, disk->info.alias)) &&
+if ((info = virHashLookup(table, entryname)) &&
info->io_status != VIR_DOMAIN_DISK_ERROR_NONE) {
if (n == nerrors)
break;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 2c3133d678..b8724b68ae 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7559,12 +7559,13 @@ qemuProcessRefreshDisks(virQEMUDriverPtr driver,
qemuDomainAsyncJob asyncJob)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
+bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV);
virHashTablePtr table = NULL;
int ret = -1;
size_t i;
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) {
-table = qemuMonitorGetBlockInfo(priv->mon, false);
+table = qemuMonitorGetBlockInfo(priv->mon, blockdev);
if (qemuDomainObjExitMonitor(driver, vm) < 0)
goto cleanup;
}
@@ -7576,8 +7577,12 @@ qemuProcessRefreshDisks(virQEMUDriverPtr driver,
virDomainDiskDefPtr disk = vm->def->disks[i];
qemuDomainDiskPrivatePtr diskpriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
struct qemuDomainDiskInfo *info;
+const char *entryname = disk->info.alias;
+
+if (blockdev)
+entryname = diskpriv->backendQomName;
-if (!(info = virHashLookup(table, disk->info.alias)))
+if (!(info = virHashLookup(table, entryname)))
continue;
if (info->removable) {
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 11/39] qemu: domain: Don't redetect backing chain when using -blockdev
We need to load the backing chain from the XML when using -blockdev.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_process.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 8dd25e2736..1d6cd72727 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6027,8 +6027,10 @@ qemuProcessPrepareHostStorage(virQEMUDriverPtr driver,
virDomainObjPtr vm,
unsigned int flags)
{
+qemuDomainObjPrivatePtr priv = vm->privateData;
size_t i;
bool cold_boot = flags & VIR_QEMU_PROCESS_START_COLD;
+bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV);
for (i = vm->def->ndisks; i > 0; i--) {
size_t idx = i - 1;
@@ -6037,7 +6039,9 @@ qemuProcessPrepareHostStorage(virQEMUDriverPtr driver,
if (virStorageSourceIsEmpty(disk->src))
continue;
-virStorageSourceBackingStoreClear(disk->src);
+/* backing chain needs to be redetected if we aren't using blockdev */
+if (!blockdev)
+virStorageSourceBackingStoreClear(disk->src);
if (qemuDomainDetermineDiskChain(driver, vm, disk, true) >= 0)
continue;
@@ -7711,7 +7715,8 @@ qemuProcessReconnect(void *opaque)
goto error;
/* backing chains need to be refreshed only if they could change */
-if (priv->reconnectBlockjobs != VIR_TRISTATE_BOOL_NO) {
+if (priv->reconnectBlockjobs != VIR_TRISTATE_BOOL_NO &&
+!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) {
/* This should be the only place that calls
* qemuDomainDetermineDiskChain with @report_broken == false
* to guarantee best-effort domain reconnect */
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 22/39] qemu: block: Add generator for the 'copy-on-read' blockdev driver
The copy on read functionality is done using a separate layer in the
backing chain. Add function to generate properties for it.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_block.c | 22 ++
src/qemu/qemu_block.h | 2 ++
2 files changed, 24 insertions(+)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
index 66e6301210..5016c9ba83 100644
--- a/src/qemu/qemu_block.c
+++ b/src/qemu/qemu_block.c
@@ -1743,3 +1743,25 @@ qemuBlockSnapshotAddLegacy(virJSONValuePtr actions,
VIR_FREE(source);
return ret;
}
+
+
+/**
+ * qemuBlockStorageGetCopyOnReadProps:
+ * @disk: disk with copy-on-read enabled
+ *
+ * Creates blockdev properties for a disk copy-on-read layer.
+ */
+virJSONValuePtr
+qemuBlockStorageGetCopyOnReadProps(virDomainDiskDefPtr disk)
+{
+qemuDomainDiskPrivatePtr priv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+virJSONValuePtr ret = NULL;
+
+ignore_value(virJSONValueObjectCreate(,
+ "s:driver", "copy-on-read",
+ "s:node-name", priv->nodeCopyOnRead,
+ "s:file", disk->src->nodeformat,
+ NULL));
+
+return ret;
+}
diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h
index fd8984e60b..62ed5027cb 100644
--- a/src/qemu/qemu_block.h
+++ b/src/qemu/qemu_block.h
@@ -67,6 +67,8 @@ qemuBlockStorageSourceGetURI(virStorageSourcePtr src);
virJSONValuePtr
qemuBlockStorageSourceGetBlockdevProps(virStorageSourcePtr src);
+virJSONValuePtr
+qemuBlockStorageGetCopyOnReadProps(virDomainDiskDefPtr disk);
typedef struct qemuBlockStorageSourceAttachData
qemuBlockStorageSourceAttachData;
typedef qemuBlockStorageSourceAttachData *qemuBlockStorageSourceAttachDataPtr;
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 14/39] qemu: domain: Add infrastructure to generate block node names
Node names for block objects in qemu need to be unique for an instance
of the qemu process. Add a counter to generate objects sequentially and
store it in the status XML so that we can restore it.
The helpers added allow to create new node names and reset the counter
after the VM process terminates.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_domain.c | 38 ++
src/qemu/qemu_domain.h | 6 ++
src/qemu/qemu_process.c | 3 +++
3 files changed, 47 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index de056272e8..6103b86478 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2438,6 +2438,9 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf,
qemuDomainObjPrivateXMLFormatPR(buf, priv);
+if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV))
+virBufferAsprintf(buf, "\n",
priv->nodenameindex);
+
if (qemuDomainObjPrivateXMLFormatBlockjobs(buf, vm) < 0)
return -1;
@@ -2933,6 +2936,14 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt,
if (qemuDomainObjPrivateXMLParseBlockjobs(priv, ctxt) < 0)
goto error;
+qemuDomainStorageIdReset(priv);
+if (virXPathULongLong("string(./nodename/@next)", ctxt,
+ >nodenameindex) == -2) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("failed to parse node name index"));
+goto error;
+}
+
return 0;
error:
@@ -13161,3 +13172,30 @@
qemuDomainGetManagedPRSocketPath(qemuDomainObjPrivatePtr priv)
return ret;
}
+
+
+/**
+ * qemuDomainStorageIdNew:
+ * @priv: qemu VM private data object.
+ *
+ * Generate a new unique id for a storage object. Useful for node name
generation.
+ */
+unsigned int
+qemuDomainStorageIdNew(qemuDomainObjPrivatePtr priv)
+{
+return ++priv->nodenameindex;
+}
+
+
+/**
+ * qemuDomainStorageIdReset:
+ * @priv: qemu VM private data object.
+ *
+ * Resets the data for the node name generator. The node names need to be
unique
+ * for a single instance, so can be reset on VM shutdown.
+ */
+void
+qemuDomainStorageIdReset(qemuDomainObjPrivatePtr priv)
+{
+priv->nodenameindex = 0;
+}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index bff293fc0a..57c130c047 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -363,6 +363,9 @@ struct _qemuDomainObjPrivate {
/* true if qemu-pr-helper process is running for the domain */
bool prDaemonRunning;
+
+/* counter for generating node names for qemu disks */
+unsigned long long nodenameindex;
};
# define QEMU_DOMAIN_PRIVATE(vm) \
@@ -1064,4 +1067,7 @@ qemuDomainDiskCachemodeFlags(int cachemode,
char * qemuDomainGetManagedPRSocketPath(qemuDomainObjPrivatePtr priv);
+unsigned int qemuDomainStorageIdNew(qemuDomainObjPrivatePtr priv);
+void qemuDomainStorageIdReset(qemuDomainObjPrivatePtr priv);
+
#endif /* __QEMU_DOMAIN_H__ */
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index fbfba63fe1..4b681a892e 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7157,6 +7157,9 @@ void qemuProcessStop(virQEMUDriverPtr driver,
/* clear all private data entries which are no longer needed */
qemuDomainObjPrivateDataClear(priv);
+/* reset node name allocator */
+qemuDomainStorageIdReset(priv);
+
/* The "release" hook cleans up additional resources */
if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
char *xml = qemuDomainDefFormatXML(driver, vm->def, 0);
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 24/39] qemu: command: format disk source commandline for -blockdev
Format the backing chain onto the commandline using the 'json' syntax
with -blockdev.
The command line formatter needs only minor tweaks to add the new
entries but we now need to initialize the strucutres that are used for
every layer of the backing chain.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_command.c | 84 -
1 file changed, 76 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index ae45c45b7f..7fa366cd3f 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2237,6 +2237,8 @@ static int
qemuBuildBlockStorageSourceAttachDataCommandline(virCommandPtr cmd,
qemuBlockStorageSourceAttachDataPtr data)
{
+char *tmp;
+
if (qemuBuildObjectCommandline(cmd, data->prmgrProps) < 0 ||
qemuBuildObjectCommandline(cmd, data->authsecretProps) < 0 ||
qemuBuildObjectCommandline(cmd, data->encryptsecretProps) < 0 ||
@@ -2246,6 +2248,22 @@
qemuBuildBlockStorageSourceAttachDataCommandline(virCommandPtr cmd,
if (data->driveCmd)
virCommandAddArgList(cmd, "-drive", data->driveCmd, NULL);
+if (data->storageProps) {
+if (!(tmp = virJSONValueToString(data->storageProps, false)))
+return -1;
+
+virCommandAddArgList(cmd, "-blockdev", tmp, NULL);
+VIR_FREE(tmp);
+}
+
+if (data->formatProps) {
+if (!(tmp = virJSONValueToString(data->formatProps, false)))
+return -1;
+
+virCommandAddArgList(cmd, "-blockdev", tmp, NULL);
+VIR_FREE(tmp);
+}
+
return 0;
}
@@ -2256,21 +2274,71 @@ qemuBuildDiskSourceCommandLine(virCommandPtr cmd,
virQEMUCapsPtr qemuCaps,
bool driveBoot)
{
-qemuBlockStorageSourceAttachDataPtr data = NULL;
+qemuBlockStorageSourceAttachDataPtr *data = NULL;
+size_t ndata = 0;
+qemuBlockStorageSourceAttachDataPtr tmp = NULL;
+virJSONValuePtr copyOnReadProps = NULL;
+virStorageSourcePtr n;
+char *str = NULL;
+size_t i;
int ret = -1;
-if (!(data = qemuBuildStorageSourceAttachPrepareDrive(disk, qemuCaps,
- driveBoot)))
-return -1;
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) {
+if (virStorageSourceIsEmpty(disk->src)) {
+ret = 0;
+goto cleanup;
+}
-if (qemuBuildStorageSourceAttachPrepareCommon(disk->src, data, qemuCaps) <
0 ||
-qemuBuildBlockStorageSourceAttachDataCommandline(cmd, data) < 0)
-goto cleanup;
+for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore)
{
+if (!(tmp = qemuBlockStorageSourceAttachPrepareBlockdev(n)))
+goto cleanup;
+
+if (qemuBuildStorageSourceAttachPrepareCommon(n, tmp, qemuCaps) <
0)
+goto cleanup;
+
+if (VIR_APPEND_ELEMENT(data, ndata, tmp) < 0)
+goto cleanup;
+}
+
+if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON &&
+!(copyOnReadProps = qemuBlockStorageGetCopyOnReadProps(disk)))
+goto cleanup;
+} else {
+if (!(tmp = qemuBuildStorageSourceAttachPrepareDrive(disk, qemuCaps,
+ driveBoot)))
+goto cleanup;
+
+if (qemuBuildStorageSourceAttachPrepareCommon(disk->src, tmp,
+ qemuCaps) < 0)
+goto cleanup;
+
+if (VIR_APPEND_ELEMENT(data, ndata, tmp) < 0)
+goto cleanup;
+}
+
+for (i = ndata; i > 0; i--) {
+if (qemuBuildBlockStorageSourceAttachDataCommandline(cmd,
+ data[i - 1]) < 0)
+goto cleanup;
+}
+
+if (copyOnReadProps) {
+if (!(str = virJSONValueToString(copyOnReadProps, false)))
+goto cleanup;
+
+virCommandAddArgList(cmd, "-blockdev", str, NULL);
+VIR_FREE(str);
+}
ret = 0;
cleanup:
-qemuBlockStorageSourceAttachDataFree(data);
+for (i = 0; i < ndata; i++)
+qemuBlockStorageSourceAttachDataFree(data[i]);
+VIR_FREE(data);
+qemuBlockStorageSourceAttachDataFree(tmp);
+virJSONValueFree(copyOnReadProps);
+VIR_FREE(str);
return ret;
}
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 13/39] conf: domain: Format out user provided backing chains in XML
If a user configures the backing chain in the XML we should not ignore
it. We already do parse it but don't format it out. As a
safety-precaution don't attempt to format detected chain into the
inactive XML.
Signed-off-by: Peter Krempa
---
src/conf/domain_conf.c | 8 ++-
.../disk-backing-chains-inactive.xml | 35 ++
.../disk-backing-chains-index-active.xml | 80 ++
.../disk-backing-chains-index-inactive.xml | 80 ++
.../disk-backing-chains-noindex-active.xml | 80 ++
.../disk-backing-chains-noindex-inactive.xml | 80 ++
tests/qemuxml2xmloutdata/disk-mirror-inactive.xml | 4 ++
.../disk-mirror-old-inactive.xml | 4 ++
8 files changed, 369 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 33b0b4ab68..55f356ef23 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23669,10 +23669,15 @@ virDomainDiskBackingStoreFormat(virBufferPtr buf,
unsigned int flags)
{
const char *format;
+bool inactive = flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE;
if (!backingStore)
return 0;
+/* don't write detected backing chain members to inactive xml */
+if (inactive && backingStore->detected)
+return 0;
+
if (backingStore->type == VIR_STORAGE_TYPE_NONE) {
virBufferAddLit(buf, "\n");
return 0;
@@ -23938,8 +23943,7 @@ virDomainDiskDefFormat(virBufferPtr buf,
/* Don't format backingStore to inactive XMLs until the code for
* persistent storage of backing chains is ready. */
-if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE) &&
-virDomainDiskBackingStoreFormat(buf, def->src->backingStore,
+if (virDomainDiskBackingStoreFormat(buf, def->src->backingStore,
xmlopt, flags) < 0)
return -1;
diff --git a/tests/qemuxml2xmloutdata/disk-backing-chains-inactive.xml
b/tests/qemuxml2xmloutdata/disk-backing-chains-inactive.xml
index a9db12ba4d..c1af58ff6f 100644
--- a/tests/qemuxml2xmloutdata/disk-backing-chains-inactive.xml
+++ b/tests/qemuxml2xmloutdata/disk-backing-chains-inactive.xml
@@ -19,6 +19,10 @@
+
+
+
+
@@ -27,6 +31,31 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
@@ -35,6 +64,7 @@
+
@@ -48,6 +78,11 @@
+
+
+
+
+
diff --git a/tests/qemuxml2xmloutdata/disk-backing-chains-index-active.xml
b/tests/qemuxml2xmloutdata/disk-backing-chains-index-active.xml
index db70ae2b53..724afa4e83 100644
--- a/tests/qemuxml2xmloutdata/disk-backing-chains-index-active.xml
+++ b/tests/qemuxml2xmloutdata/disk-backing-chains-index-active.xml
@@ -19,6 +19,10 @@
+
+
+
+
@@ -27,6 +31,31 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
@@ -35,6 +64,7 @@
+
@@ -48,6 +78,11 @@
+
+
+
+
+
@@ -60,6 +95,51 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git
[libvirt] [PATCH RFC 21/39] qemu: proces: assign node names for user defined backing chains
---
src/qemu/qemu_domain.c | 76 --
1 file changed, 74 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c98be208f1..526634d819 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -137,6 +137,14 @@ static virClassPtr qemuDomainSaveCookieClass;
static void qemuDomainLogContextDispose(void *obj);
static void qemuDomainSaveCookieDispose(void *obj);
+
+static int
+qemuDomainPrepareStorageSourceBlockdev(virDomainDiskDefPtr disk,
+ virStorageSourcePtr src,
+ qemuDomainObjPrivatePtr priv,
+ virQEMUDriverConfigPtr cfg);
+
+
static int
qemuDomainOnceInit(void)
{
@@ -8638,6 +8646,10 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
if (qemuDomainPrepareDiskSourceData(disk, n, cfg, priv->qemuCaps) < 0)
goto cleanup;
+
+if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV) &&
+qemuDomainPrepareStorageSourceBlockdev(disk, n, priv, cfg) < 0)
+goto cleanup;
}
ret = 0;
@@ -13085,6 +13097,61 @@ qemuDomainPrepareDiskSourceLegacy(virDomainDiskDefPtr
disk,
}
+static int
+qemuDomainPrepareStorageSourceBlockdev(virDomainDiskDefPtr disk,
+ virStorageSourcePtr src,
+ qemuDomainObjPrivatePtr priv,
+ virQEMUDriverConfigPtr cfg)
+{
+src->id = qemuDomainStorageIdNew(priv);
+
+if (virAsprintf(>nodestorage, "libvirt-%u-storage", src->id) < 0 ||
+virAsprintf(>nodeformat, "libvirt-%u-format", src->id) < 0)
+return -1;
+
+if (qemuDomainValidateStorageSource(src, priv->qemuCaps) < 0)
+return -1;
+
+if (qemuDomainPrepareDiskSourceData(disk, src, cfg, priv->qemuCaps) < 0)
+return -1;
+
+if (qemuDomainSecretStorageSourcePrepare(priv, src,
+ src->nodestorage,
+ src->nodeformat) < 0)
+return -1;
+
+if (qemuDomainPrepareStorageSourcePR(disk->src, priv, src->nodestorage) <
0)
+return -1;
+
+if (qemuDomainPrepareStorageSourceTLS(disk->src, cfg, src->nodestorage,
+ priv->qemuCaps) < 0)
+return -1;
+
+return 0;
+}
+
+
+static int
+qemuDomainPrepareDiskSourceBlockdev(virDomainDiskDefPtr disk,
+qemuDomainObjPrivatePtr priv,
+virQEMUDriverConfigPtr cfg)
+{
+qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+virStorageSourcePtr n;
+
+if (disk->copy_on_read == VIR_TRISTATE_SWITCH_ON &&
+virAsprintf(>nodeCopyOnRead, "libvirt-CoR-%s", disk->dst) <
0)
+return -1;
+
+for (n = disk->src; virStorageSourceIsBacking(n); n = n->backingStore) {
+if (qemuDomainPrepareStorageSourceBlockdev(disk, n, priv, cfg) < 0)
+return -1;
+}
+
+return 0;
+}
+
+
int
qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk,
qemuDomainObjPrivatePtr priv,
@@ -13092,8 +13159,13 @@ qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk,
{
qemuDomainPrepareDiskCachemode(disk);
-if (qemuDomainPrepareDiskSourceLegacy(disk, priv, cfg) < 0)
-return -1;
+if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV)) {
+if (qemuDomainPrepareDiskSourceBlockdev(disk, priv, cfg) < 0)
+return -1;
+} else {
+if (qemuDomainPrepareDiskSourceLegacy(disk, priv, cfg) < 0)
+return -1;
+}
return 0;
}
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 15/39] conf: Implement private data formatting and parsing for disks
Allow storing of private data in the status XML for disks.
Signed-off-by: Peter Krempa
---
src/conf/domain_conf.c | 60 ++
src/conf/domain_conf.h | 7 ++
2 files changed, 67 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 55f356ef23..cf52f9af5f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9523,6 +9523,30 @@ virDomainDiskDefDriverParseXML(virDomainDiskDefPtr def,
}
+static int
+virDomainDiskDefParsePrivateData(xmlXPathContextPtr ctxt,
+ virDomainDiskDefPtr disk,
+ virDomainXMLOptionPtr xmlopt)
+{
+xmlNodePtr save_node = ctxt->node;
+int ret = 0;
+
+if (!xmlopt ||
+!xmlopt->privateData.diskParse)
+return 0;
+
+if (!(ctxt->node = virXPathNode("./privateData", ctxt)))
+goto cleanup;
+
+if (xmlopt->privateData.diskParse(ctxt, disk) < 0)
+ret = -1;
+
+ cleanup:
+ctxt->node = save_node;
+return ret;
+}
+
+
#define VENDOR_LEN 8
#define PRODUCT_LEN 16
@@ -9938,6 +9962,10 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
goto error;
}
+if (flags & VIR_DOMAIN_DEF_PARSE_STATUS &&
+virDomainDiskDefParsePrivateData(ctxt, def, xmlopt) < 0)
+goto error;
+
if (virDomainDiskDefParseValidate(def, vmSeclabels, nvmSeclabels) < 0)
goto error;
@@ -23878,6 +23906,35 @@ virDomainDiskDefFormatMirror(virBufferPtr buf,
}
+static int
+virDomainDiskDefFormatPrivateData(virBufferPtr buf,
+ virDomainDiskDefPtr disk,
+ unsigned int flags,
+ virDomainXMLOptionPtr xmlopt)
+{
+virBuffer childBuf = VIR_BUFFER_INITIALIZER;
+
+if (!(flags & VIR_DOMAIN_DEF_FORMAT_STATUS) ||
+!xmlopt ||
+!xmlopt->privateData.diskFormat)
+return 0;
+
+virBufferSetChildIndent(, buf);
+
+if (xmlopt->privateData.diskFormat(disk, ) < 0)
+goto error;
+
+if (virXMLFormatElement(buf, "privateData", NULL, ) < 0)
+goto error;
+
+return 0;
+
+ error:
+virBufferFreeAndReset();
+return -1;
+}
+
+
static int
virDomainDiskDefFormat(virBufferPtr buf,
virDomainDiskDefPtr def,
@@ -23991,6 +24048,9 @@ virDomainDiskDefFormat(virBufferPtr buf,
virDomainDeviceInfoFormat(buf, >info,
flags | VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT);
+if (virDomainDiskDefFormatPrivateData(buf, def, flags, xmlopt) < 0)
+return -1;
+
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "\n");
return 0;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 5e2f21dea3..68c24e0748 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2735,6 +2735,11 @@ typedef int
(*virDomainXMLPrivateDataParseFunc)(xmlXPathContextPtr,
typedef void *(*virDomainXMLPrivateDataGetParseOpaqueFunc)(virDomainObjPtr vm);
+typedef int (*virDomainXMLPrivateDataDiskParseFunc)(xmlXPathContextPtr ctxt,
+virDomainDiskDefPtr disk);
+typedef int (*virDomainXMLPrivateDataDiskFormatFunc)(virDomainDiskDefPtr disk,
+ virBufferPtr buf);
+
typedef int
(*virDomainXMLPrivateDataStorageSourceParseFunc)(xmlXPathContextPtr ctxt,
virStorageSourcePtr src);
typedef int
(*virDomainXMLPrivateDataStorageSourceFormatFunc)(virStorageSourcePtr src,
@@ -2749,6 +2754,8 @@ struct _virDomainXMLPrivateDataCallbacks {
/* note that private data for devices are not copied when using
* virDomainDefCopy and similar functions */
virDomainXMLPrivateDataNewFuncdiskNew;
+virDomainXMLPrivateDataDiskParseFunc diskParse;
+virDomainXMLPrivateDataDiskFormatFunc diskFormat;
virDomainXMLPrivateDataNewFuncvcpuNew;
virDomainXMLPrivateDataNewFuncchrSourceNew;
virDomainXMLPrivateDataNewFuncvsockNew;
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 04/39] tests: qemu: Drop disk from hostdev-mdev tests
The disk is not necessary to test the mdevs. Signed-off-by: Peter Krempa --- tests/qemuxml2argvdata/hostdev-mdev-display-missing-graphics.xml| 6 -- .../hostdev-mdev-display-spice-egl-headless.x86_64-latest.args | 2 -- tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.xml | 6 -- .../hostdev-mdev-display-spice-opengl.x86_64-latest.args| 2 -- tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.xml| 6 -- .../hostdev-mdev-display-vnc-egl-headless.x86_64-latest.args| 2 -- tests/qemuxml2argvdata/hostdev-mdev-display-vnc-egl-headless.xml| 6 -- tests/qemuxml2argvdata/hostdev-mdev-display-vnc.x86_64-latest.args | 2 -- tests/qemuxml2argvdata/hostdev-mdev-display-vnc.xml | 6 -- tests/qemuxml2argvdata/hostdev-mdev-display.xml | 6 -- tests/qemuxml2argvdata/hostdev-mdev-invalid-target-address.xml | 5 - tests/qemuxml2argvdata/hostdev-mdev-precreated.args | 2 -- tests/qemuxml2argvdata/hostdev-mdev-precreated.xml | 6 -- tests/qemuxml2argvdata/hostdev-mdev-src-address-invalid.xml | 6 -- tests/qemuxml2xmloutdata/hostdev-mdev-display.xml | 6 -- tests/qemuxml2xmloutdata/hostdev-mdev-precreated.xml| 6 -- 16 files changed, 75 deletions(-) diff --git a/tests/qemuxml2argvdata/hostdev-mdev-display-missing-graphics.xml b/tests/qemuxml2argvdata/hostdev-mdev-display-missing-graphics.xml index ea559a6444..55b60ba133 100644 --- a/tests/qemuxml2argvdata/hostdev-mdev-display-missing-graphics.xml +++ b/tests/qemuxml2argvdata/hostdev-mdev-display-missing-graphics.xml @@ -14,12 +14,6 @@ destroy /usr/bin/qemu-system-i686 - - - - - - diff --git a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.x86_64-latest.args b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.x86_64-latest.args index 0ac90c81d2..b84869264e 100644 --- a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.x86_64-latest.args +++ b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.x86_64-latest.args @@ -23,8 +23,6 @@ file=/tmp/lib/domain--1-QEMUGuest2/master-key.aes \ -no-acpi \ -boot strict=on \ -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ --drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \ --device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ -spice port=0,seamless-migration=on \ -display egl-headless \ -device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,\ diff --git a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.xml b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.xml index c8f10c2f3a..3a686ad2bf 100644 --- a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.xml +++ b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-egl-headless.xml @@ -14,12 +14,6 @@ destroy /usr/bin/qemu-system-i686 - - - - - - diff --git a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.x86_64-latest.args b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.x86_64-latest.args index 1fd9fdaa16..80c56abfb9 100644 --- a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.x86_64-latest.args +++ b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.x86_64-latest.args @@ -23,8 +23,6 @@ file=/tmp/lib/domain--1-QEMUGuest2/master-key.aes \ -no-acpi \ -boot strict=on \ -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ --drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-0-0 \ --device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ -spice port=0,gl=on,rendernode=/dev/dri/foo,seamless-migration=on \ -device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,\ vram64_size_mb=0,vgamem_mb=16,max_outputs=1,bus=pci.0,addr=0x2 \ diff --git a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.xml b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.xml index 18c9817608..a632e58a41 100644 --- a/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.xml +++ b/tests/qemuxml2argvdata/hostdev-mdev-display-spice-opengl.xml @@ -14,12 +14,6 @@ destroy /usr/bin/qemu-system-i686 - - - - - - diff --git a/tests/qemuxml2argvdata/hostdev-mdev-display-vnc-egl-headless.x86_64-latest.args b/tests/qemuxml2argvdata/hostdev-mdev-display-vnc-egl-headless.x86_64-latest.args index cdf545d0e0..91708d7663 100644 --- a/tests/qemuxml2argvdata/hostdev-mdev-display-vnc-egl-headless.x86_64-latest.args +++ b/tests/qemuxml2argvdata/hostdev-mdev-display-vnc-egl-headless.x86_64-latest.args @@ -23,8 +23,6 @@ file=/tmp/lib/domain--1-QEMUGuest2/master-key.aes \ -no-acpi \ -boot strict=on \ -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ --drive
[libvirt] [PATCH RFC 07/39] qemu: hotplug: Don't generate alias when detaching disk
It should be impossible to lack an alias in the domain definition. Other
disk types don't generate it so remove it here as well.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_hotplug.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 1488f0a7c2..1538abf155 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -4902,11 +4902,6 @@ qemuDomainDetachVirtioDiskDevice(virQEMUDriverPtr driver,
goto cleanup;
}
-if (!detach->info.alias) {
-if (qemuAssignDeviceDiskAlias(vm->def, detach) < 0)
-goto cleanup;
-}
-
if (!async)
qemuDomainMarkDeviceForRemoval(vm, >info);
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 10/39] qemu: process: clear QEMU_CAPS_BLOCKDEV for VMs where we can't support it
SD cards are currently passed by using -drive only which would not be
compatible with using -blockdev fully.
Floppies at least in the case of the i440 machine type don't have a
reasonable qdev ID if -drive is not used and thus would not allow
queries and other operations. Since floppy drives are obsolete anyways
clear blockdev when using them.
Clear QEMU_CAPS_BLOCKDEV if the VM has such devices.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_process.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 27bd8b9465..8dd25e2736 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5884,6 +5884,16 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver,
qemuProcessPrepareAllowReboot(vm);
+/* clear the 'blockdev' capability for VMs which have disks that need
+ * -drive or which have floppies where we can't reliably get the QOM path
*/
+for (i = 0; i < vm->def->ndisks; i++) {
+if (qemuDiskBusNeedsDriveArg(vm->def->disks[i]->bus) ||
+vm->def->disks[i]->bus == VIR_DOMAIN_DISK_BUS_FDC) {
+virQEMUCapsClear(priv->qemuCaps, QEMU_CAPS_BLOCKDEV);
+break;
+}
+}
+
/*
* Normally PCI addresses are assigned in the virDomainCreate
* or virDomainDefine methods. We might still need to assign
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 19/39] qemu: alias: Generate QDEV name of the block backend for disks
When we stop using -drive qemu stops reporting it in some of the monitor
commands. To allow referring the disk frontends and the corresponding
block backends we need to know these names. Unfortunately different
buses require different names.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_alias.c | 86 +++--
src/qemu/qemu_alias.h | 3 +-
src/qemu/qemu_hotplug.c | 2 +-
3 files changed, 65 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index 80d9b6cf46..be3663f8d2 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -175,44 +175,82 @@ qemuAssignDeviceControllerAlias(virDomainDefPtr domainDef,
}
-/* Our custom -drive naming scheme used with id= */
int
qemuAssignDeviceDiskAlias(virDomainDefPtr def,
- virDomainDiskDefPtr disk)
+ virDomainDiskDefPtr disk,
+ virQEMUCapsPtr qemuCaps)
{
+qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
const char *prefix = virDomainDiskBusTypeToString(disk->bus);
int controllerModel = -1;
if (disk->info.alias)
return 0;
-if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) {
-if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) {
-controllerModel = qemuDomainFindSCSIControllerModel(def,
->info);
-if (controllerModel < 0)
+if (!disk->info.alias) {
+if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) {
+if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) {
+controllerModel = qemuDomainFindSCSIControllerModel(def,
+
>info);
+if (controllerModel < 0)
+return -1;
+}
+
+if (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI ||
+controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) {
+if (virAsprintf(>info.alias, "%s%d-%d-%d", prefix,
+disk->info.addr.drive.controller,
+disk->info.addr.drive.bus,
+disk->info.addr.drive.unit) < 0)
+return -1;
+} else {
+if (virAsprintf(>info.alias, "%s%d-%d-%d-%d", prefix,
+disk->info.addr.drive.controller,
+disk->info.addr.drive.bus,
+disk->info.addr.drive.target,
+disk->info.addr.drive.unit) < 0)
+return -1;
+}
+} else {
+int idx = virDiskNameToIndex(disk->dst);
+if (virAsprintf(>info.alias, "%s-disk%d", prefix, idx) < 0)
return -1;
}
+}
-if (disk->bus != VIR_DOMAIN_DISK_BUS_SCSI ||
-controllerModel == VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LSILOGIC) {
-if (virAsprintf(>info.alias, "%s%d-%d-%d", prefix,
-disk->info.addr.drive.controller,
-disk->info.addr.drive.bus,
-disk->info.addr.drive.unit) < 0)
+/* For -blockdev we need to know the QDEV ids of the block backend of the
+ * disk. The QDEV id used by qemu is based on the alias so we generate them
+ * here. */
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BLOCKDEV)) {
+switch ((virDomainDiskBus) disk->bus) {
+case VIR_DOMAIN_DISK_BUS_IDE:
+case VIR_DOMAIN_DISK_BUS_SATA:
+case VIR_DOMAIN_DISK_BUS_SCSI:
+if (VIR_STRDUP(diskPriv->backendQomName, disk->info.alias) < 0)
return -1;
-} else {
-if (virAsprintf(>info.alias, "%s%d-%d-%d-%d", prefix,
-disk->info.addr.drive.controller,
-disk->info.addr.drive.bus,
-disk->info.addr.drive.target,
-disk->info.addr.drive.unit) < 0)
+break;
+
+case VIR_DOMAIN_DISK_BUS_VIRTIO:
+if (virAsprintf(>backendQomName,
+"/machine/peripheral/%s/virtio-backend",
+disk->info.alias) < 0)
return -1;
+break;
+
+case VIR_DOMAIN_DISK_BUS_USB:
+if (virAsprintf(>backendQomName,
+"/machine/peripheral/%s/%s.0/legacy[0]",
+disk->info.alias, disk->info.alias) < 0)
+return -1;
+break;
+
+case VIR_DOMAIN_DISK_BUS_FDC:
+case VIR_DOMAIN_DISK_BUS_XEN:
+case VIR_DOMAIN_DISK_BUS_UML:
+case VIR_DOMAIN_DISK_BUS_SD:
+case VIR_DOMAIN_DISK_BUS_LAST:
+break;
}
-} else {
-int idx = virDiskNameToIndex(disk->dst);
-if
[libvirt] [PATCH RFC 08/39] util: virqemu: Simplify debugging if building QOM object with missing args
Print the values so it's simpler to debug.
Signed-off-by: Peter Krempa
---
src/util/virqemu.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/util/virqemu.c b/src/util/virqemu.c
index 30b8dc18d4..7ffa9f780e 100644
--- a/src/util/virqemu.c
+++ b/src/util/virqemu.c
@@ -248,8 +248,9 @@ virQEMUBuildObjectCommandlineFromJSONInternal(virBufferPtr
buf,
virJSONValuePtr props)
{
if (!type || !alias) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("missing 'type' or 'alias' field of QOM 'object'"));
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("missing 'type'(%s) or 'alias'(%s) field of QOM
'object'"),
+ NULLSTR(type), NULLSTR(alias));
return -1;
}
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 09/39] qemu: caps: Add capability for using the blockdev infrastructure
The capability currently is not enabled so that we can add individual
bits first.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_capabilities.c | 1 +
src/qemu/qemu_capabilities.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 0fb800589a..6b4c14ac50 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -507,6 +507,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
/* 315 */
"vfio-pci.display",
+ "blockdev",
);
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 9e8ad5f5c3..452761e672 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -491,6 +491,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
syntax-check */
/* 315 */
QEMU_CAPS_VFIO_PCI_DISPLAY, /* -device vfio-pci.display */
+QEMU_CAPS_BLOCKDEV, /* -blockdev and blockdev-add are supported */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 01/39] qemu: monitor: Reuse qemuMonitorJSONQueryBlock in qemuMonitorJSONBlockIoThrottleInfo
The wrapper executes the command and does error detection so there's no
need to open-code all of those things.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_monitor_json.c | 37 +
1 file changed, 5 insertions(+), 32 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 75d0738b5d..abbcede097 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4833,21 +4833,14 @@ int qemuMonitorJSONOpenGraphics(qemuMonitorPtr mon,
goto cleanup; \
}
static int
-qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr result,
+qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr io_throttle,
const char *device,
virDomainBlockIoTuneInfoPtr reply)
{
-virJSONValuePtr io_throttle;
int ret = -1;
size_t i;
bool found = false;
-if (!(io_throttle = virJSONValueObjectGetArray(result, "return"))) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _(" block_io_throttle reply was missing device list"));
-goto cleanup;
-}
-
for (i = 0; i < virJSONValueArraySize(io_throttle); i++) {
virJSONValuePtr temp_dev = virJSONValueArrayGet(io_throttle, i);
virJSONValuePtr inserted;
@@ -5017,33 +5010,13 @@ int qemuMonitorJSONGetBlockIoThrottle(qemuMonitorPtr
mon,
virDomainBlockIoTuneInfoPtr reply)
{
int ret = -1;
-virJSONValuePtr cmd = NULL;
-virJSONValuePtr result = NULL;
+virJSONValuePtr devices = NULL;
-cmd = qemuMonitorJSONMakeCommand("query-block", NULL);
-if (!cmd)
+if (!(devices = qemuMonitorJSONQueryBlock(mon)))
return -1;
-if (qemuMonitorJSONCommand(mon, cmd, ) < 0)
-goto cleanup;
-
-if (virJSONValueObjectHasKey(result, "error")) {
-if (qemuMonitorJSONHasError(result, "DeviceNotActive"))
-virReportError(VIR_ERR_OPERATION_INVALID,
- _("No active operation on device: %s"), device);
-else if (qemuMonitorJSONHasError(result, "NotSupported"))
-virReportError(VIR_ERR_OPERATION_INVALID,
- _("Operation is not supported for device: %s"),
device);
-else
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Unexpected error"));
-goto cleanup;
-}
-
-ret = qemuMonitorJSONBlockIoThrottleInfo(result, device, reply);
- cleanup:
-virJSONValueFree(cmd);
-virJSONValueFree(result);
+ret = qemuMonitorJSONBlockIoThrottleInfo(devices, device, reply);
+virJSONValueFree(devices);
return ret;
}
--
2.16.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC 05/39] tests: qemuxml2argv: Fork CAPS_LATEST test cases for 'blockdev'
The blockdev support will change existing approach to add disks to VMs so all tests using the DO_TEST_CAPS_LATEST approach which have any disks need to be forked so that the changes can be applied. Signed-off-by: Peter Krempa --- tests/qemuxml2argvdata/disk-aio.x86_64-2.12.0.args | 37 + .../qemuxml2argvdata/disk-cache.x86_64-2.12.0.args | 50 + .../disk-cdrom-network.x86_64-2.12.0.args | 41 ++ .../disk-cdrom-tray.x86_64-2.12.0.args | 39 ++ .../qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args | 35 .../disk-copy_on_read.x86_64-2.12.0.args | 41 ++ .../disk-detect-zeroes.x86_64-2.12.0.args | 37 + .../disk-error-policy.x86_64-2.12.0.args | 41 ++ .../disk-floppy.x86_64-2.12.0.args | 35 .../disk-network-gluster.x86_64-2.12.0.args| 44 +++ .../disk-network-iscsi.x86_64-2.12.0.args | 63 ++ .../disk-network-nbd.x86_64-2.12.0.args| 46 .../disk-network-rbd.x86_64-2.12.0.args| 61 + .../disk-network-sheepdog.x86_64-2.12.0.args | 35 .../disk-network-source-auth.x86_64-2.12.0.args| 47 .../disk-network-tlsx509.x86_64-2.12.0.args| 59 .../disk-readonly-disk.x86_64-2.12.0.args | 34 .../disk-shared.x86_64-2.12.0.args | 37 + ...isk-virtio-scsi-reservations.x86_64-2.12.0.args | 43 +++ .../floppy-drive-fat.x86_64-2.12.0.args| 33 tests/qemuxml2argvtest.c | 20 +++ 21 files changed, 878 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-aio.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-cache.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-cdrom-network.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-cdrom-tray.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-cdrom.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-copy_on_read.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-detect-zeroes.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-error-policy.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-floppy.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-network-gluster.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-network-iscsi.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-network-nbd.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-network-rbd.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-network-sheepdog.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-network-source-auth.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-network-tlsx509.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-readonly-disk.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-shared.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-virtio-scsi-reservations.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/floppy-drive-fat.x86_64-2.12.0.args diff --git a/tests/qemuxml2argvdata/disk-aio.x86_64-2.12.0.args b/tests/qemuxml2argvdata/disk-aio.x86_64-2.12.0.args new file mode 100644 index 00..1dfade0882 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-aio.x86_64-2.12.0.args @@ -0,0 +1,37 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-machine pc-i440fx-2.12,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-realtime mlock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-drive file=/dev/HostVG/QEMUGuest1,format=qcow2,if=none,id=drive-ide0-0-0,\ +cache=none,aio=native \ +-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1,\ +write-cache=on \ +-drive file=/dev/HostVG/QEMUGuest2,format=raw,if=none,id=drive-ide0-1-0,\ +readonly=on,aio=threads \ +-device ide-cd,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/disk-cache.x86_64-2.12.0.args
[libvirt] [PATCH RFC 06/39] tests: qemu: Add test data for backing chains and indexes
Add test data for nested backing chains with/without indexes (used in status XMLs) which will excercise blockdev and the related work. Signed-off-by: Peter Krempa --- .../disk-backing-chains-index.x86_64-2.12.0.args | 1 + .../disk-backing-chains-index.x86_64-latest.args | 1 + .../qemuxml2argvdata/disk-backing-chains-index.xml | 145 + .../disk-backing-chains-noindex.x86_64-2.12.0.args | 58 + .../disk-backing-chains-noindex.x86_64-latest.args | 58 + .../disk-backing-chains-noindex.xml| 145 + tests/qemuxml2argvtest.c | 4 + .../disk-backing-chains-index-active.xml | 76 +++ .../disk-backing-chains-index-inactive.xml | 76 +++ .../disk-backing-chains-noindex-active.xml | 76 +++ .../disk-backing-chains-noindex-inactive.xml | 76 +++ tests/qemuxml2xmltest.c| 2 + 12 files changed, 718 insertions(+) create mode 12 tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-2.12.0.args create mode 12 tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-backing-chains-index.xml create mode 100644 tests/qemuxml2argvdata/disk-backing-chains-noindex.x86_64-2.12.0.args create mode 100644 tests/qemuxml2argvdata/disk-backing-chains-noindex.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/disk-backing-chains-noindex.xml create mode 100644 tests/qemuxml2xmloutdata/disk-backing-chains-index-active.xml create mode 100644 tests/qemuxml2xmloutdata/disk-backing-chains-index-inactive.xml create mode 100644 tests/qemuxml2xmloutdata/disk-backing-chains-noindex-active.xml create mode 100644 tests/qemuxml2xmloutdata/disk-backing-chains-noindex-inactive.xml diff --git a/tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-2.12.0.args b/tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-2.12.0.args new file mode 12 index 00..3f4cd9040d --- /dev/null +++ b/tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-2.12.0.args @@ -0,0 +1 @@ +disk-backing-chains-noindex.x86_64-2.12.0.args \ No newline at end of file diff --git a/tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-latest.args b/tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-latest.args new file mode 12 index 00..549eb65512 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-backing-chains-index.x86_64-latest.args @@ -0,0 +1 @@ +disk-backing-chains-noindex.x86_64-latest.args \ No newline at end of file diff --git a/tests/qemuxml2argvdata/disk-backing-chains-index.xml b/tests/qemuxml2argvdata/disk-backing-chains-index.xml new file mode 100644 index 00..95b8a64cf8 --- /dev/null +++ b/tests/qemuxml2argvdata/disk-backing-chains-index.xml @@ -0,0 +1,145 @@ + + QEMUGuest1 + c7a5fdbd-edaf-9455-926a-d65c16db1809 + 219136 + 219136 + 1 + +hvm + + + + destroy + restart + destroy + +/usr/bin/qemu-system-i686 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/qemuxml2argvdata/disk-backing-chains-noindex.x86_64-2.12.0.args b/tests/qemuxml2argvdata/disk-backing-chains-noindex.x86_64-2.12.0.args new file mode 100644 index 00..dea109b13a --- /dev/null +++
[libvirt] [PATCH RFC 03/39] qemu: monitor: Allow using 'qdev' instead of 'device' for getting disk throttling
The 'device' field reported by 'query-block' is empty when -blockdev is
used. Add an argument which will allow matching disk by using the qdev
id so we can use this code with -blockdev.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_driver.c | 2 +-
src/qemu/qemu_monitor.c | 8 +---
src/qemu/qemu_monitor.h | 3 ++-
src/qemu/qemu_monitor_json.c | 21 ++---
src/qemu/qemu_monitor_json.h | 3 ++-
tests/qemumonitorjsontest.c | 2 +-
6 files changed, 25 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ae762a3189..b81ad7cdbc 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18623,7 +18623,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom,
if (!(device = qemuAliasDiskDriveFromDisk(disk)))
goto endjob;
qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, );
+ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, NULL, );
if (qemuDomainObjExitMonitor(driver, vm) < 0)
goto endjob;
if (ret < 0)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 84310ff8ca..ac9cde4577 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3469,14 +3469,16 @@ qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon,
int
qemuMonitorGetBlockIoThrottle(qemuMonitorPtr mon,
- const char *device,
+ const char *drivealias,
+ const char *qdevid,
virDomainBlockIoTuneInfoPtr reply)
{
-VIR_DEBUG("device=%p, reply=%p", device, reply);
+VIR_DEBUG("drivealias=%s, qdevid=%s, reply=%p",
+ NULLSTR(drivealias), NULLSTR(qdevid), reply);
QEMU_CHECK_MONITOR(mon);
-return qemuMonitorJSONGetBlockIoThrottle(mon, device, reply);
+return qemuMonitorJSONGetBlockIoThrottle(mon, drivealias, qdevid, reply);
}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 01860f11f4..a25b1f54ea 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -941,7 +941,8 @@ int qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon,
bool supportMaxLengthOptions);
int qemuMonitorGetBlockIoThrottle(qemuMonitorPtr mon,
- const char *device,
+ const char *drivealias,
+ const char *qdevid,
virDomainBlockIoTuneInfoPtr reply);
int qemuMonitorSystemWakeup(qemuMonitorPtr mon);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index fc65198f6f..f33535327c 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4834,7 +4834,8 @@ int qemuMonitorJSONOpenGraphics(qemuMonitorPtr mon,
}
static int
qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr io_throttle,
- const char *device,
+ const char *drivealias,
+ const char *qdevid,
virDomainBlockIoTuneInfoPtr reply)
{
int ret = -1;
@@ -4844,7 +4845,8 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr
io_throttle,
for (i = 0; i < virJSONValueArraySize(io_throttle); i++) {
virJSONValuePtr temp_dev = virJSONValueArrayGet(io_throttle, i);
virJSONValuePtr inserted;
-const char *current_dev;
+const char *current_drive;
+const char *current_qdev;
if (!temp_dev || virJSONValueGetType(temp_dev) !=
VIR_JSON_TYPE_OBJECT) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -4853,14 +4855,18 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr
io_throttle,
goto cleanup;
}
-if (!(current_dev = virJSONValueObjectGetString(temp_dev, "device"))) {
+current_qdev = virJSONValueObjectGetString(temp_dev, "qdev");
+current_drive = virJSONValueObjectGetString(temp_dev, "device");
+
+if (!current_drive && !current_qdev) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("block_io_throttle device entry "
"was not in expected format"));
goto cleanup;
}
-if (STRNEQ(current_dev, device))
+if ((drivealias && STRNEQ(current_drive, drivealias)) ||
+(qdevid && STRNEQ(current_qdev, qdevid)))
continue;
found = true;
@@ -4901,7 +4907,7 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr
io_throttle,
if (!found) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot find throttling info for device '%s'"),
- device);
+ drivealias ? drivealias : qdevid);
goto cleanup;
}
ret = 0;
@@ -5012,7 +5018,8 @@ int
[libvirt] [PATCH RFC 02/39] qemu: monitor: Allow using 'id' instead of 'device' for 'block_set_io_throttle'
The 'device' argument matches only the legacy drive alias. For blockdev
we need to set the throttling for a QOM id and thus we'll need to use
the 'id' field.
Signed-off-by: Peter Krempa
---
src/qemu/qemu_driver.c | 2 +-
src/qemu/qemu_monitor.c | 8 +---
src/qemu/qemu_monitor.h | 3 ++-
src/qemu/qemu_monitor_json.c | 14 ++
src/qemu/qemu_monitor_json.h | 3 ++-
tests/qemumonitorjsontest.c | 2 +-
6 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index fb0d4a8c7a..ae762a3189 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18494,7 +18494,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom,
* via the JSON error code from the block_set_io_throttle call */
qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorSetBlockIoThrottle(priv->mon, device,
+ret = qemuMonitorSetBlockIoThrottle(priv->mon, device, NULL,
, supportMaxOptions,
set_fields &
QEMU_BLOCK_IOTUNE_SET_GROUP_NAME,
supportMaxLengthOptions);
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 6e0644221b..84310ff8ca 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3448,17 +3448,19 @@ qemuMonitorGetBlockJobInfo(qemuMonitorPtr mon,
int
qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon,
- const char *device,
+ const char *drivealias,
+ const char *qomid,
virDomainBlockIoTuneInfoPtr info,
bool supportMaxOptions,
bool supportGroupNameOption,
bool supportMaxLengthOptions)
{
-VIR_DEBUG("device=%p, info=%p", device, info);
+VIR_DEBUG("drivealias=%s, qomid=%s, info=%p",
+ NULLSTR(drivealias), NULLSTR(qomid), info);
QEMU_CHECK_MONITOR(mon);
-return qemuMonitorJSONSetBlockIoThrottle(mon, device, info,
+return qemuMonitorJSONSetBlockIoThrottle(mon, drivealias, qomid, info,
supportMaxOptions,
supportGroupNameOption,
supportMaxLengthOptions);
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 2fa8d5b51d..01860f11f4 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -933,7 +933,8 @@ int qemuMonitorOpenGraphics(qemuMonitorPtr mon,
bool skipauth);
int qemuMonitorSetBlockIoThrottle(qemuMonitorPtr mon,
- const char *device,
+ const char *drivealias,
+ const char *qomid,
virDomainBlockIoTuneInfoPtr info,
bool supportMaxOptions,
bool supportGroupNameOption,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index abbcede097..fc65198f6f 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -4913,7 +4913,8 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr
io_throttle,
#undef GET_THROTTLE_STATS_OPTIONAL
int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon,
- const char *device,
+ const char *drivealias,
+ const char *qomid,
virDomainBlockIoTuneInfoPtr info,
bool supportMaxOptions,
bool supportGroupNameOption,
@@ -4923,12 +4924,17 @@ int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr
mon,
virJSONValuePtr cmd = NULL;
virJSONValuePtr result = NULL;
virJSONValuePtr args = NULL;
+const char *errdev = drivealias;
+
+if (!errdev)
+errdev = qomid;
if (!(cmd = qemuMonitorJSONMakeCommand("block_set_io_throttle", NULL)))
return -1;
if (virJSONValueObjectCreate(,
- "s:device", device,
+ "S:device", drivealias,
+ "S:id", qomid,
"U:bps", info->total_bytes_sec,
"U:bps_rd", info->read_bytes_sec,
"U:bps_wr", info->write_bytes_sec,
@@ -4983,10 +4989,10 @@ int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr
mon,
if (virJSONValueObjectHasKey(result, "error")) {
if (qemuMonitorJSONHasError(result, "DeviceNotActive")) {
virReportError(VIR_ERR_OPERATION_INVALID,
- _("No active operation on device: %s"), device);
+
[libvirt] [PATCH RFC 00/39] qemu: Add support for -blockdev
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}}
Returns no reasonable data:
{
"stats": {
"flush_total_time_ns": 0,
"wr_highest_offset": 0,
"wr_total_time_ns": 0,
"failed_wr_operations": 0,
"failed_rd_operations": 0,
"wr_merged": 0,
"wr_bytes": 0,
"timed_stats": [
],
"failed_flush_operations": 0,
"account_invalid": false,
"rd_total_time_ns": 0,
"flush_operations": 0,
"wr_operations": 0,
"rd_merged": 0,
"rd_bytes": 0,
"invalid_flush_operations": 0,
"account_failed": false,
"rd_operations": 0,
"invalid_wr_operations": 0,
"invalid_rd_operations": 0
},
"node-name": "libvirt-7-storage"
},
{
"parent": {
"stats": {
"flush_total_time_ns": 0,
"wr_highest_offset": 0,
"wr_total_time_ns": 0,
"failed_wr_operations": 0,
"failed_rd_operations": 0,
"wr_merged": 0,
"wr_bytes": 0,
"timed_stats": [
],
"failed_flush_operations": 0,
"account_invalid": false,
"rd_total_time_ns": 0,
"flush_operations": 0,
"wr_operations": 0,
"rd_merged": 0,
"rd_bytes": 0,
"invalid_flush_operations": 0,
"account_failed": false,
"rd_operations": 0,
"invalid_wr_operations": 0,
"invalid_rd_operations": 0
},
"node-name": "libvirt-7-storage"
},
"stats": {
"flush_total_time_ns": 0,
"wr_highest_offset": 0,
"wr_total_time_ns": 0,
"failed_wr_operations": 0,
"failed_rd_operations": 0,
"wr_merged": 0,
"wr_bytes": 0,
"timed_stats": [
],
"failed_flush_operations": 0,
"account_invalid": false,
"rd_total_time_ns": 0,
"flush_operations": 0,
"wr_operations": 0,
"rd_merged": 0,
"rd_bytes": 0,
"invalid_flush_operations": 0,
"account_failed": false,
"rd_operations": 0,
"invalid_wr_operations": 0,
"invalid_rd_operations": 0
},
"node-name": "libvirt-7-format"
},
the 'libvirt-7-storage' and 'libvirt-7-format' nodes represent the ISO
backing the CDROM used to boot the VM so reads were executed.
In the old approach when we use -drive and query-nodes is false the
output looks like this:
{
"device": "drive-ide0-0-0",
"parent": {
"stats": {
"flush_total_time_ns": 0,
"wr_highest_offset": 0,
"wr_total_time_ns": 0,
"failed_wr_operations": 0,
"failed_rd_operations": 0,
"wr_merged": 0,
"wr_bytes": 0,
"timed_stats": [
],
"failed_flush_operations": 0,
"account_invalid": false,
"rd_total_time_ns": 0,
"flush_operations": 0,
"wr_operations": 0,
"rd_merged": 0,
"rd_bytes": 0,
"invalid_flush_operations": 0,
"account_failed": false,
"rd_operations": 0,
"invalid_wr_operations": 0,
"invalid_rd_operations": 0
},
"node-name": "#block080"
},
"stats": {
"flush_total_time_ns": 0,
"wr_highest_offset": 0,
"wr_total_time_ns": 0,
"failed_wr_operations": 0,
"failed_rd_operations": 0,
"wr_merged": 0,
"wr_bytes": 0,
"timed_stats": [
],
"failed_flush_operations": 0,
"account_invalid": true,
"rd_total_time_ns": 204236271,
"flush_operations": 0,
"wr_operations": 0,
"rd_merged": 0,
"rd_bytes": 30046628,
"invalid_flush_operations": 0,
"account_failed": true,
"idle_time_ns": 18766797619,
"rd_operations": 14680,
"invalid_wr_operations": 0,
"invalid_rd_operations": 0
},
"node-name": "#block152"
},
I also get all zeroes when I use 'query-nodes' true on a machine started
with -drive.
Without the stats we'd not achieve feature parity unfortunately.
Kevin, could you please have a look?
Peter Krempa (39):
qemu: monitor: Reuse qemuMonitorJSONQueryBlock in
qemuMonitorJSONBlockIoThrottleInfo
qemu: monitor: Allow using 'id' instead of 'device' for
'block_set_io_throttle'
qemu: monitor: Allow using 'qdev' instead of 'device' for getting disk
throttling
tests: qemu: Drop disk from
Re: [libvirt] [PATCH 1/2] socket: preserve real errno when socket/bind calls fail
On Tue, Jul 24, 2018 at 05:51:50PM +0200, Jiri Denemark wrote:
> On Tue, Jul 24, 2018 at 15:18:09 +0100, Daniel P. Berrangé wrote:
> > When reporting socket/bind failures we want to ensure any fatal error
> > reported is as accurate as possible. We'll prefer reporting a bind()
> > errno over a socket() errno, because if socket() works but bind() fails
> > that is a more significant event.
> >
> > Signed-off-by: Daniel P. Berrangé
> > ---
> > src/rpc/virnetsocket.c | 24
> > 1 file changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
> > index fee61ace60..8e04d61e98 100644
> > --- a/src/rpc/virnetsocket.c
> > +++ b/src/rpc/virnetsocket.c
> > @@ -310,8 +310,8 @@ int virNetSocketNewListenTCP(const char *nodename,
> > struct addrinfo hints;
> > int fd = -1;
> > size_t i;
> > -bool addrInUse = false;
> > -bool familyNotSupported = false;
> > +int socketErrno = 0;
> > +int bindErrno = 0;
> > virSocketAddr tmp_addr;
> >
> > *retsocks = NULL;
> > @@ -351,7 +351,7 @@ int virNetSocketNewListenTCP(const char *nodename,
> > if ((fd = socket(runp->ai_family, runp->ai_socktype,
> > runp->ai_protocol)) < 0) {
> > if (errno == EAFNOSUPPORT) {
> > -familyNotSupported = true;
> > +socketErrno = errno;
> > runp = runp->ai_next;
> > continue;
> > }
> > @@ -386,7 +386,7 @@ int virNetSocketNewListenTCP(const char *nodename,
> > virReportSystemError(errno, "%s", _("Unable to bind to
> > port"));
> > goto error;
> > }
> > -addrInUse = true;
> > +bindErrno = errno;
> > VIR_FORCE_CLOSE(fd);
> > runp = runp->ai_next;
> > continue;
> > @@ -409,14 +409,14 @@ int virNetSocketNewListenTCP(const char *nodename,
> > fd = -1;
> > }
> >
> > -if (nsocks == 0 && familyNotSupported) {
> > -virReportSystemError(EAFNOSUPPORT, "%s", _("Unable to bind to
> > port"));
> > -goto error;
> > -}
> > -
> > -if (nsocks == 0 &&
> > -addrInUse) {
> > -virReportSystemError(EADDRINUSE, "%s", _("Unable to bind to
> > port"));
> > +if (nsocks == 0) {
> > +if (bindErrno) {
> > +virReportSystemError(bindErrno, "%s", _("Unable to bind to
> > port"));
> > +} else if (socketErrno) {
> > +virReportSystemError(socketErrno, "%s", _("Unable to create
> > socket"));
> > +} else {
> > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("No addresses
> > to bind to"));
> > +}
>
> We usually don't use {} around single line bodies, you can remove all of
> them here.
Oh yes, I always get this wrong since QEMU requires the opposite
to libvirt - always {} no matter what :-)
>
> > goto error;
> > }
>
> Reviewed-by: Jiri Denemark
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 RESEND 00/12] PCI passthrough support on s390
On Wed, 2018-07-25 at 16:34 +0800, Yi Min Zhao wrote: > 在 2018/7/24 下午11:43, Andrea Bolognani 写道: > > > > More concrete questions: one of the zPCI test cases includes > > > > > > > > > > > > > > > > > > > > > > > >> > > function='0x0'/> > > > > > > > > > > > function='0x0' uid='0x' fid='0x'/> > > > > > > > > > > > > which translates to > > > > > > > > -device zpci,uid=3,fid=2,target=pci.1,id=zpci3 \ > > > > -device pci-bridge,chassis_nr=1,id=pci.1,bus=pci.0,addr=0x1 \ > > > > -device zpci,uid=65535,fid=4294967295,target=hostdev0,id=zpci65535 \ > > > > -device vfio-pci,host=:00:00.0,id=hostdev0,bus=pci.1,addr=0x1f \ > > > > > > > > How does the pci-bridge controller show up in the guest, if at all? > > Qemu hides pci-bridge devices and just exposes pci devices to the guest. > In above example, indeed, qemu will generate a pci-bridge device and it will > be existing in pci topology. But the guest can't see it. This is very > special. Yeah, that's kinda problematic as it violates the principle of least surprise... If s390 guests can only see a flat PCI topology, then we should find a way to reject bridges altogether instead of allowing the user to create them (or even create them automatically) only for them not to show up in the guest. > > > > Do the bus= and addr= attributes of vfio-pci and pci-bridge in the > > > > example above matter eg. for migration purposes? > > Do you mean we leave attribute generation for bus and addr to qemu? That would be the idea, but of course it can only work if the address of the underlying PCI device can change without affecting the guest in any way, including migration. If that's not the case, and the PCI address needs to be as stable as the IDs, then I don't think there's a way to avoid storing it in the guest XML, no matter how confusing that will end up looking. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 1/2] util: Add helper APIs to get/verify VF Representor name
Thanks again John for giving detailed review and feedback for v3 patchset.
I have tried incorporating the suggestions in v4.
v4: https://www.redhat.com/archives/libvir-list/2018-June/msg01807.html
-Jai
From: John Ferlan
Sent: 13 April 2018 00:38
To: Jai Singh Rana; libvir-list@redhat.com
Cc: Rana, JaiSingh
Subject: Re: [libvirt] [PATCH v3 1/2] util: Add helper APIs to get/verify VF
Representor name
On 04/04/2018 12:29 PM, Jai Singh Rana wrote:
> Switchdev VF representor interface name on host is queried based on
> Bus:Device:Function information of pci SR-IOV device in Domain's
> 'hostdev' structure and subsequently verifying the required net sysfs
> directory and file entries of VF representor according to switchdev
> model.
You are missing the S-o-b:
The someone new policy is that:
Contributors to libvirt projects must assert that they are in compliance
with the Developer Certificate of Origin 1.1. This is achieved by adding
a "Signed-off-by" line containing the contributor's name and e-mail to
every commit message. The presence of this line attests that the
contributor has read the above lined DCO and agrees with its statements.
https://developercertificate.org/
> ---
> v3 includes changes based on v2's[1] feedback and suggestions. Fixes
> warnings reported by syntax-check.
> [1] https://www.redhat.com/archives/libvir-list/2018-February/msg00562.html
>
> po/POTFILES.in | 1 +
> src/libvirt_private.syms| 7 +
> src/util/Makefile.inc.am| 2 +
> src/util/virhostdev.c | 2 +-
> src/util/virhostdev.h | 8 +
> src/util/virnetdevhostdev.c | 374
>
> src/util/virnetdevhostdev.h | 35 +
> 7 files changed, 428 insertions(+), 1 deletion(-)
> create mode 100644 src/util/virnetdevhostdev.c
> create mode 100644 src/util/virnetdevhostdev.h
>
You probably don't have cppi installed, but if you did it would have
pointed out a few more syntax-check issues...
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index d84859a4e..8cd6b86e8 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -234,6 +234,7 @@ src/util/virmdev.c
> src/util/virnetdev.c
> src/util/virnetdevbandwidth.c
> src/util/virnetdevbridge.c
> +src/util/virnetdevhostdev.c
> src/util/virnetdevip.c
> src/util/virnetdevmacvlan.c
> src/util/virnetdevmidonet.c
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index f6897915c..fad235206 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1923,6 +1923,7 @@ virHostCPUStatsAssign;
> virHostdevFindUSBDevice;
> virHostdevIsSCSIDevice;
> virHostdevManagerGetDefault;
> +virHostdevNetDevice;
> virHostdevPCINodeDeviceDetach;
> virHostdevPCINodeDeviceReAttach;
> virHostdevPCINodeDeviceReset;
> @@ -2306,6 +2307,12 @@ virNetDevBridgeSetSTPDelay;
> virNetDevBridgeSetVlanFiltering;
>
>
> +# util/virnetdevhostdev.h
> +virNetdevHostdevCheckVFRIfName;
> +virNetdevHostdevGetVFRIfName;
> +virNetdevHostdevVFRIfStats;
> +
> +
> # util/virnetdevip.h
> virNetDevIPAddrAdd;
> virNetDevIPAddrDel;
> diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am
> index a3c3b711f..31fe11c68 100644
> --- a/src/util/Makefile.inc.am
> +++ b/src/util/Makefile.inc.am
> @@ -104,6 +104,8 @@ UTIL_SOURCES = \
>util/virnetdevbandwidth.h \
>util/virnetdevbridge.c \
>util/virnetdevbridge.h \
> + util/virnetdevhostdev.c \
> + util/virnetdevhostdev.h \
>util/virnetdevip.c \
>util/virnetdevip.h \
>util/virnetdevmacvlan.c \
> diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
> index a12224c58..4f7b46a04 100644
> --- a/src/util/virhostdev.c
> +++ b/src/util/virhostdev.c
> @@ -306,7 +306,7 @@ virHostdevIsVirtualFunction(virDomainHostdevDefPtr
> hostdev)
> }
>
>
> -static int
> +int
> virHostdevNetDevice(virDomainHostdevDefPtr hostdev,
> int pfNetDevIdx,
> char **linkdev,
> diff --git a/src/util/virhostdev.h b/src/util/virhostdev.h
> index 54e1c66be..735220add 100644
> --- a/src/util/virhostdev.h
> +++ b/src/util/virhostdev.h
> @@ -60,6 +60,14 @@ struct _virHostdevManager {
> };
>
> virHostdevManagerPtr virHostdevManagerGetDefault(void);
> +
> +int
> +virHostdevNetDevice(virDomainHostdevDefPtr hostdev,
> +int pfNetDevIdx,
> +char **linkdev,
> +int *vf)
> +ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(4);
> +
> int
> virHostdevPreparePCIDevices(virHostdevManagerPtr hostdev_mgr,
> const char *drv_name,
> diff --git a/src/util/virnetdevhostdev.c b/src/util/virnetdevhostdev.c
> new file mode 100644
> index 0..19f95bfdd
> --- /dev/null
> +++ b/src/util/virnetdevhostdev.c
> @@ -0,0 +1,374 @@
> +/*
> + * virnetdevhostdev.c: utilities to get/verify Switchdev VF Representor
> + *
> + * This library is free software; you can redistribute it and/or
> + *
Re: [libvirt] [PATCHv3] replace 'if' type conditions with 'switch' for VIR_NETWORK_FORWARD_
On Wednesday, July 25, 2018 at 8:37 PM, Erik wrote:
> On Tue, Jul 24, 2018 at 11:49:48AM +0800, Shi Lei wrote:
> > Signed-off-by: Shi Lei
> > ---
> >
> > v2 here: https://www.redhat.com/archives/libvir-list/2018-July/msg01423.html
> > since v2:
> > - typecast def->forward.type to virNetworkForwardType explicitly
> > in all the switches rather than change its type to enum in
> > the struct definition
> >
> > v1 here: https://www.redhat.com/archives/libvir-list/2018-July/msg01314.html
> > since v1:
> > - Change the type declaration of _virNetworkForwardDef.type
> > from int to virNetworkForwardType
> > - use the default case to report out of range error with
> > virReportEnumRangeError
> >
> ...
>
> > +if (virNetworkObjIsActive(obj)) {
> > +switch ((virNetworkForwardType) def->forward.type) {
> > +case VIR_NETWORK_FORWARD_NONE:
> > +case VIR_NETWORK_FORWARD_NAT:
> > +case VIR_NETWORK_FORWARD_ROUTE:
> > +/* Only three of the L3 network types that are configured by
> > + * libvirt need to have iptables rules reloaded. The 4th L3
> > + * network type, forward='open', doesn't need this because it
> > + * has no iptables rules.
> > + */
> > +networkRemoveFirewallRules(def);
> > +/* No need to check return value since already logged
> > internally */
>
> I dropped ^this commentary, adjusted the commit message and pushed the patch.
Sorry, I forgot to remove this commentary.
>
> Congratulations on your first libvirt patch.
> Erik
>
Thanks!
Shi Lei
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 3/3] qemu: Use the correct vm def on cold attach
On Wed, Jul 25, 2018 at 08:39:26AM -0400, John Ferlan wrote: On 07/25/2018 06:40 AM, Michal Privoznik wrote: On 07/16/2018 11:14 PM, John Ferlan wrote: https://bugzilla.redhat.com/show_bug.cgi?id=1559867 When attaching a device to the domain we need to be sure to use the correct domain definition (vm->def or vm->newDef) when calling virDomainDeviceDefParse because the post parse processing algorithms that may assign an address for the device will use whatever domain definition was passed in. This effectively reduces AttachDevice(AFFECT_LIVE | AFFECT_CONFIG) to two subsequent AttachDevice calls, thus possibly attaching two different devices, making the AFFECT_CONFIG option pointless. E.g. when hotplugging a network interface, it might end up both on a different PCI slot and with a different MAC address, which I'm afraid might break some use cases. Additionally, some devices (SCSI hostdev and SCSI disk) use algorithms that rely on knowing what already exists of the other type when generating the new device's address. Using the wrong VM definition could result in duplicated addresses. If suitablity for both live and persistent definition is a problem, the address allocation code should take both domain definitions into account and generate a single address for both device copies. In the case of the bz, two hostdev's with no domain address provided were added to the running domain's config only. However, the parsing algorithm used the live domain in order to figure out the host device address resulting in the same address being used and a subsequent start failing due to duplicate address. Fix this by separating the checks/code into CONFIG and LIVE processing using the correct definition for each block and performing cleanup for both options as necessary. Signed-off-by: John Ferlan --- src/qemu/qemu_driver.c | 52 +++--- 1 file changed, 23 insertions(+), 29 deletions(-) As I said in my review of v2 I still consider this a waste of energy and we might possibly end up having to revert this change because it breaks someone's use case. Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 RESEND 03/12] conf: Introduce a new PCI address extension flag
On Wed, 2018-07-25 at 16:58 +0800, Yi Min Zhao wrote:
[...]
> > > +case VIR_DOMAIN_DEVICE_CONTROLLER:
> > > +case VIR_DOMAIN_DEVICE_DISK:
> > > +case VIR_DOMAIN_DEVICE_LEASE:
> > > +case VIR_DOMAIN_DEVICE_FS:
> > > +case VIR_DOMAIN_DEVICE_NET:
> > > +case VIR_DOMAIN_DEVICE_INPUT:
> > > +case VIR_DOMAIN_DEVICE_SOUND:
> > > +case VIR_DOMAIN_DEVICE_VIDEO:
> > > +case VIR_DOMAIN_DEVICE_HOSTDEV:
> > > +case VIR_DOMAIN_DEVICE_WATCHDOG:
> > > +case VIR_DOMAIN_DEVICE_GRAPHICS:
> > > +case VIR_DOMAIN_DEVICE_HUB:
> > > +case VIR_DOMAIN_DEVICE_REDIRDEV:
> > > +case VIR_DOMAIN_DEVICE_SMARTCARD:
> > > +case VIR_DOMAIN_DEVICE_MEMBALLOON:
> > > +case VIR_DOMAIN_DEVICE_NVRAM:
> > > +case VIR_DOMAIN_DEVICE_RNG:
> > > +case VIR_DOMAIN_DEVICE_SHMEM:
> > > +case VIR_DOMAIN_DEVICE_TPM:
> > > +case VIR_DOMAIN_DEVICE_PANIC:
> > > +case VIR_DOMAIN_DEVICE_MEMORY:
> > > +case VIR_DOMAIN_DEVICE_IOMMU:
> > > +case VIR_DOMAIN_DEVICE_VSOCK:
> >
> > Did you validate that all of the above can be used with zPCI?
>
> Yes, I did. But how far can zPCI be used is a question. I make sure that
> if their address
> type can be PCI type zPCI address could be expanded. But we don't
> guarantee they can
> be really used in qemu. Like RNG device can't be used because it doesn't
> support MSIx
> which is required on S390.
If that's the case, I'm not sure the current solution is what we
want: if someone creates a guest and includes a RNG device in the
configuration, they probably expect it to, well, work. IIUC, with
the current implementation they would get a non-working RNG device
instead, which certainly feels suboptimal.
> > [...]
> > > +static virDomainPCIAddressExtensionFlags
> > > +qemuDomainDeviceCalculatePCIAddressExtensionFlags(virQEMUCapsPtr
> > > qemuCaps,
> > > + virDomainDeviceDefPtr
> > > dev)
> > > +{
> > > +virDomainPCIAddressExtensionFlags extFlags = 0;
> > > +
> > > +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI) &&
> > > +qemuDomainDeviceSupportZPCI(dev))
> > > +extFlags |= VIR_PCI_ADDRESS_EXTENSION_ZPCI;
> >
> > The libvirt code style guidelines[1] state that this should be
> > formatted as
> >
> >if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI) &&
> >qemuDomainDeviceSupportZPCI(dev)) {
> >extFlags |= VIR_PCI_ADDRESS_EXTENSION_ZPCI;
> >}
> >
> > [1] https://libvirt.org/hacking.html
>
> I see a lot of code in libvirt use this style. Is it new guideline?
It's certainly been around for the past 3+ years. There's a lot of
code in libvirt that's *way* older than that, though :)
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/5] Couple of LXC fixes & improvements
*** BLURB HERE *** Michal Prívozník (5): lxc: Refresh capabilities on virConnectGetCapabilities lxc: Report supported huge pages lxc: Enable under valgrind again lxc: Don't leak @veths in virLXCProcessStart lxc: Don't mangle @cfg refs in virLXCProcessBuildControllerCmd src/lxc/lxc_conf.c| 4 src/lxc/lxc_driver.c | 13 + src/lxc/lxc_process.c | 15 +-- 3 files changed, 14 insertions(+), 18 deletions(-) -- 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/5] lxc: Don't leak @veths in virLXCProcessStart
The individual strings are freed, but the array is never freed. 8 bytes in 1 blocks are definitely lost in loss record 28 of 1,098 at 0x4C2CE3F: malloc (vg_replace_malloc.c:298) by 0x4C2F1BF: realloc (vg_replace_malloc.c:785) by 0x52C9C92: virReallocN (viralloc.c:245) by 0x52C9D88: virExpandN (viralloc.c:294) by 0x23414D99: virLXCProcessSetupInterfaces (lxc_process.c:552) by 0x23417457: virLXCProcessStart (lxc_process.c:1356) by 0x2341F71C: lxcDomainCreateWithFiles (lxc_driver.c:1088) by 0x2341F805: lxcDomainCreate (lxc_driver.c:1123) by 0x55917EB: virDomainCreate (libvirt-domain.c:6534) by 0x1367D1: remoteDispatchDomainCreate (remote_daemon_dispatch_stubs.h:4434) by 0x1366EA: remoteDispatchDomainCreateHelper (remote_daemon_dispatch_stubs.h:4410) by 0x546FDF1: virNetServerProgramDispatchCall (virnetserverprogram.c:437) Signed-off-by: Michal Privoznik --- src/lxc/lxc_process.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 14502e12fe..3e44da1aaf 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -1559,6 +1559,7 @@ int virLXCProcessStart(virConnectPtr conn, virCommandFree(cmd); for (i = 0; i < nveths; i++) VIR_FREE(veths[i]); +VIR_FREE(veths); for (i = 0; i < nttyFDs; i++) VIR_FORCE_CLOSE(ttyFDs[i]); VIR_FREE(ttyFDs); -- 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/5] lxc: Don't mangle @cfg refs in virLXCProcessBuildControllerCmd
The config object is refed but unrefed only on error which leaves
refcount unbalanced on successful return.
Signed-off-by: Michal Privoznik
---
src/lxc/lxc_process.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 3e44da1aaf..d021a890f7 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -931,7 +931,7 @@ virLXCProcessBuildControllerCmd(virLXCDriverPtr driver,
filterstr = virLogGetFilters();
if (!filterstr) {
virReportOOMError();
-goto cleanup;
+goto error;
}
virCommandAddEnvPair(cmd, "LIBVIRT_LOG_FILTERS", filterstr);
@@ -943,7 +943,7 @@ virLXCProcessBuildControllerCmd(virLXCDriverPtr driver,
outputstr = virLogGetOutputs();
if (!outputstr) {
virReportOOMError();
-goto cleanup;
+goto error;
}
virCommandAddEnvPair(cmd, "LIBVIRT_LOG_OUTPUTS", outputstr);
@@ -973,7 +973,7 @@ virLXCProcessBuildControllerCmd(virLXCDriverPtr driver,
char *tmp = NULL;
if (virAsprintf(, "--share-%s",
nsInfoLocal[i]) < 0)
-goto cleanup;
+goto error;
virCommandAddArg(cmd, tmp);
virCommandAddArgFormat(cmd, "%d", nsInheritFDs[i]);
virCommandPassFD(cmd, nsInheritFDs[i], 0);
@@ -999,11 +999,13 @@ virLXCProcessBuildControllerCmd(virLXCDriverPtr driver,
* write the live domain status XML with the PID */
virCommandRequireHandshake(cmd);
+ cleanup:
+virObjectUnref(cfg);
return cmd;
- cleanup:
+ error:
virCommandFree(cmd);
-virObjectUnref(cfg);
-return NULL;
+cmd = NULL;
+goto cleanup;
}
--
2.16.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/5] lxc: Enable under valgrind again
So we originally disabled LXC driver when libvirtd is running
under valgrind back in 05436ab7ff087 (which dates to beginning of
2009) as it was causing valgrind to crash. It's not the case
anymore. Valgrind works with LXC happily.
Signed-off-by: Michal Privoznik
---
src/lxc/lxc_driver.c | 11 ---
1 file changed, 11 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 6969dddcab..8867645cdc 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1550,19 +1550,8 @@ static int lxcStateInitialize(bool privileged,
void *opaque ATTRIBUTE_UNUSED)
{
virCapsPtr caps = NULL;
-const char *ld;
virLXCDriverConfigPtr cfg = NULL;
-/* Valgrind gets very annoyed when we clone containers, so
- * disable LXC when under valgrind
- * XXX remove this when valgrind is fixed
- */
-ld = virGetEnvBlockSUID("LD_PRELOAD");
-if (ld && strstr(ld, "vgpreload")) {
-VIR_INFO("Running under valgrind, disabling driver");
-return 0;
-}
-
/* Check that the user is root, silently disable if not */
if (!privileged) {
VIR_INFO("Not running privileged, disabling driver");
--
2.16.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] lxc: Refresh capabilities on virConnectGetCapabilities
While not as critical as in qemu driver, there are still some
runtime information we report in capabilities XML that might
change throughout time. For instance, onlined CPUs (which affects
reported L3 cache sizes).
Signed-off-by: Michal Privoznik
---
src/lxc/lxc_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 9b329269a9..6969dddcab 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -205,7 +205,7 @@ static char *lxcConnectGetCapabilities(virConnectPtr conn) {
if (virConnectGetCapabilitiesEnsureACL(conn) < 0)
return NULL;
-if (!(caps = virLXCDriverGetCapabilities(driver, false)))
+if (!(caps = virLXCDriverGetCapabilities(driver, true)))
return NULL;
xml = virCapabilitiesFormatXML(caps);
--
2.16.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/5] lxc: Report supported huge pages
There are two places where we report supported sizes of huge pages:
/capabilities/host/cpu/pages
/capabilities/host/topology/cells/cell/pages
The former aggregates sizes over all NUMA nodes while the latter
reports supported sizes only for given node. While we are
reporting per NUMA node sizes we are not reporting the aggregated
sizes. I've noticed this when wondering why doesn't allocpages
completer work.
Signed-off-by: Michal Privoznik
---
src/lxc/lxc_conf.c | 4
1 file changed, 4 insertions(+)
diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c
index 5cd6f231dd..949bfe246a 100644
--- a/src/lxc/lxc_conf.c
+++ b/src/lxc/lxc_conf.c
@@ -86,6 +86,10 @@ virCapsPtr virLXCDriverCapsInit(virLXCDriverPtr driver)
if (driver && virNodeSuspendGetTargetMask(>host.powerMgmt) < 0)
VIR_WARN("Failed to get host power management capabilities");
+/* Add huge pages info */
+if (virCapabilitiesInitPages(caps) < 0)
+VIR_WARN("Failed to get pages info");
+
if (virGetHostUUID(caps->host.host_uuid)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("cannot get the host uuid"));
--
2.16.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [go PATCH] travis: update some of the test matrix versions to moderize coverage
On Wed, 2018-07-25 at 13:26 +0100, Daniel P. Berrangé wrote: > > > go: > > >- 1.5 > > > - - 1.6 > > >- 1.7 > > > - - 1.8 > > >- 1.9 > > > + - 1.10 > > Sigh, this gets interpreted as a number, which is 1.1, so I need to > add quotes to force it to be a string. Fair enough... Please add quotes around all of them for consistency's sake while at it. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 3/3] qemu: Use the correct vm def on cold attach
On 07/25/2018 06:40 AM, Michal Privoznik wrote:
> On 07/16/2018 11:14 PM, John Ferlan wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1559867
>>
>> When attaching a device to the domain we need to be sure
>> to use the correct domain definition (vm->def or vm->newDef)
>> when calling virDomainDeviceDefParse because the post parse
>> processing algorithms that may assign an address for the
>> device will use whatever domain definition was passed in.
>>
>> Additionally, some devices (SCSI hostdev and SCSI disk) use
>> algorithms that rely on knowing what already exists of the
>> other type when generating the new device's address. Using
>> the wrong VM definition could result in duplicated addresses.
>>
>> In the case of the bz, two hostdev's with no domain address
>> provided were added to the running domain's config only.
>> However, the parsing algorithm used the live domain in
>> order to figure out the host device address resulting in
>> the same address being used and a subsequent start failing
>> due to duplicate address.
>>
>> Fix this by separating the checks/code into CONFIG and LIVE
>> processing using the correct definition for each block and
>> performing cleanup for both options as necessary.
>>
>> Signed-off-by: John Ferlan
>> ---
>> src/qemu/qemu_driver.c | 52 +++---
>> 1 file changed, 23 insertions(+), 29 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 7d519c0714..3dbd5c62d2 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -8473,7 +8473,8 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
>> {
>> virDomainDefPtr vmdef = NULL;
>> virQEMUDriverConfigPtr cfg = NULL;
>> -virDomainDeviceDefPtr dev = NULL, dev_copy = NULL;
>> +virDomainDeviceDefPtr devConf = NULL;
>> +virDomainDeviceDefPtr devLive = NULL;
>> int ret = -1;
>> virCapsPtr caps = NULL;
>> unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
>> @@ -8487,49 +8488,43 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr
>> vm,
>> if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
>> goto cleanup;
>>
>> -dev = dev_copy = virDomainDeviceDefParse(xml, vm->def,
>> - caps, driver->xmlopt,
>> - parse_flags);
>> -if (dev == NULL)
>> -goto cleanup;
>> -
>> -if (virDomainDeviceValidateAliasForHotplug(vm, dev, flags) < 0)
>> -goto cleanup;
>> -
>> -if (flags & VIR_DOMAIN_AFFECT_CONFIG &&
>> -flags & VIR_DOMAIN_AFFECT_LIVE) {
>> -/* If we are affecting both CONFIG and LIVE
>> - * create a deep copy of device as adding
>> - * to CONFIG takes one instance.
>> - */
>> -dev_copy = virDomainDeviceDefCopy(dev, vm->def, caps,
>> driver->xmlopt);
>> -if (!dev_copy)
>> -goto cleanup;
>> -}
>> -
>> +/* The config and live post processing address auto-generation
>> algorithms
>> + * rely on the correct vm->def or vm->newDef being passed, so call the
>> + * device parse based on which definition is in use */
>> if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
>> -/* Make a copy for updated domain. */
>> vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt);
>> if (!vmdef)
>> goto cleanup;
>>
>> -if (virDomainDefCompatibleDevice(vmdef, dev, NULL,
>> +if (!(devConf = virDomainDeviceDefParse(xml, vmdef, caps,
>> +driver->xmlopt,
>> parse_flags)))
>> +goto cleanup;
>> +
>> +if (virDomainDefCompatibleDevice(vmdef, devConf, NULL,
>> VIR_DOMAIN_DEVICE_ACTION_ATTACH,
>> false) < 0)
>> goto cleanup;
>> -if ((ret = qemuDomainAttachDeviceConfig(vmdef, dev, caps,
>> +
>> +if ((ret = qemuDomainAttachDeviceConfig(vmdef, devConf, caps,
>> parse_flags,
>> driver->xmlopt)) < 0)
>> goto cleanup;
>> }
>>
>> if (flags & VIR_DOMAIN_AFFECT_LIVE) {
>> -if (virDomainDefCompatibleDevice(vm->def, dev_copy, NULL,
>> +if (!(devLive = virDomainDeviceDefParse(xml, vm->def, caps,
>> +driver->xmlopt,
>> parse_flags)))
>> +goto cleanup;
>> +
>> +if (virDomainDeviceValidateAliasForHotplug(vm, devLive, flags) < 0)
>> +goto cleanup;
>> +
>> +if (virDomainDefCompatibleDevice(vm->def, devLive, NULL,
>> VIR_DOMAIN_DEVICE_ACTION_ATTACH,
>> true) < 0)
>> goto cleanup;
>>
>> -if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, driver)) <
Re: [libvirt] [PATCHv3] replace 'if' type conditions with 'switch' for VIR_NETWORK_FORWARD_
On Tue, Jul 24, 2018 at 11:49:48AM +0800, Shi Lei wrote:
> Signed-off-by: Shi Lei
> ---
>
> v2 here: https://www.redhat.com/archives/libvir-list/2018-July/msg01423.html
> since v2:
> - typecast def->forward.type to virNetworkForwardType explicitly
> in all the switches rather than change its type to enum in
> the struct definition
>
> v1 here: https://www.redhat.com/archives/libvir-list/2018-July/msg01314.html
> since v1:
> - Change the type declaration of _virNetworkForwardDef.type
> from int to virNetworkForwardType
> - use the default case to report out of range error with
> virReportEnumRangeError
>
...
> @@ -2128,20 +2150,37 @@ networkReloadFirewallRulesHelper(virNetworkObjPtr obj,
>
> virObjectLock(obj);
> def = virNetworkObjGetDef(obj);
> -if (virNetworkObjIsActive(obj) &&
> -((def->forward.type == VIR_NETWORK_FORWARD_NONE) ||
> - (def->forward.type == VIR_NETWORK_FORWARD_NAT) ||
> - (def->forward.type == VIR_NETWORK_FORWARD_ROUTE))) {
> -/* Only three of the L3 network types that are configured by
> - * libvirt need to have iptables rules reloaded. The 4th L3
> - * network type, forward='open', doesn't need this because it
> - * has no iptables rules.
> - */
> -networkRemoveFirewallRules(def);
> -if (networkAddFirewallRules(def) < 0) {
> -/* failed to add but already logged */
> +if (virNetworkObjIsActive(obj)) {
> +switch ((virNetworkForwardType) def->forward.type) {
> +case VIR_NETWORK_FORWARD_NONE:
> +case VIR_NETWORK_FORWARD_NAT:
> +case VIR_NETWORK_FORWARD_ROUTE:
> +/* Only three of the L3 network types that are configured by
> + * libvirt need to have iptables rules reloaded. The 4th L3
> + * network type, forward='open', doesn't need this because it
> + * has no iptables rules.
> + */
> +networkRemoveFirewallRules(def);
> +/* No need to check return value since already logged internally
> */
I dropped ^this commentary, adjusted the commit message and pushed the patch.
Congratulations on your first libvirt patch.
Erik
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [go PATCH] travis: update some of the test matrix versions to moderize coverage
On Wed, Jul 25, 2018 at 02:12:26PM +0200, Andrea Bolognani wrote: > On Wed, 2018-07-25 at 12:58 +0100, Daniel P. Berrangé wrote: > > Add latest go 1.10 to matrix and (arbitrarily) trim out 1.6 and 1.8 > > > > Add libvirt 4.5.0 and (arbitrarily) trim out 1.2.10 > > > > Signed-off-by: Daniel P. Berrangé > > --- > > .travis.yml | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/.travis.yml b/.travis.yml > > index 1bdb48f..7cd7587 100644 > > --- a/.travis.yml > > +++ b/.travis.yml > > @@ -5,17 +5,16 @@ sudo: require > > > > go: > >- 1.5 > > - - 1.6 > >- 1.7 > > - - 1.8 > >- 1.9 > > + - 1.10 Sigh, this gets interpreted as a number, which is 1.1, so I need to add quotes to force it to be a string. > > > > env: > >- LIBVIRT=1.2.0 EXT=gz > > - - LIBVIRT=1.2.10 EXT=gz > >- LIBVIRT=1.2.20 EXT=gz > >- LIBVIRT=2.5.0 EXT=xz > >- LIBVIRT=3.6.0 EXT=xz > > + - LIBVIRT=4.5.0 EXT=xz > > Looks sane enough, so I'll (arbitrarily) give it a > > Reviewed-by: Andrea Bolognani > > -- > Andrea Bolognani / Red Hat / Virtualization > 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] [go PATCH] travis: update some of the test matrix versions to moderize coverage
On Wed, 2018-07-25 at 12:58 +0100, Daniel P. Berrangé wrote: > Add latest go 1.10 to matrix and (arbitrarily) trim out 1.6 and 1.8 > > Add libvirt 4.5.0 and (arbitrarily) trim out 1.2.10 > > Signed-off-by: Daniel P. Berrangé > --- > .travis.yml | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/.travis.yml b/.travis.yml > index 1bdb48f..7cd7587 100644 > --- a/.travis.yml > +++ b/.travis.yml > @@ -5,17 +5,16 @@ sudo: require > > go: >- 1.5 > - - 1.6 >- 1.7 > - - 1.8 >- 1.9 > + - 1.10 > > env: >- LIBVIRT=1.2.0 EXT=gz > - - LIBVIRT=1.2.10 EXT=gz >- LIBVIRT=1.2.20 EXT=gz >- LIBVIRT=2.5.0 EXT=xz >- LIBVIRT=3.6.0 EXT=xz > + - LIBVIRT=4.5.0 EXT=xz Looks sane enough, so I'll (arbitrarily) give it a Reviewed-by: Andrea Bolognani -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: Fix virStorageBackendIQNFound() to work on FreeBSD
On Wed, 2018-07-25 at 07:57 -0400, John Ferlan wrote: > Should have gone with the overly intrusive virStringSplit[Count] option > like I suggested during review ;-) [which is at least partially open > coded here, but I digress]. I didn't follow the review, I just jumped on it when I spotted the build failures on CentOS CI... If you can come up with a better version that still works on FreeBSD I'm pretty sure nobody will mind :) -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [go PATCH] travis: update some of the test matrix versions to moderize coverage
Add latest go 1.10 to matrix and (arbitrarily) trim out 1.6 and 1.8 Add libvirt 4.5.0 and (arbitrarily) trim out 1.2.10 Signed-off-by: Daniel P. Berrangé --- .travis.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 1bdb48f..7cd7587 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,17 +5,16 @@ sudo: require go: - 1.5 - - 1.6 - 1.7 - - 1.8 - 1.9 + - 1.10 env: - LIBVIRT=1.2.0 EXT=gz - - LIBVIRT=1.2.10 EXT=gz - LIBVIRT=1.2.20 EXT=gz - LIBVIRT=2.5.0 EXT=xz - LIBVIRT=3.6.0 EXT=xz + - LIBVIRT=4.5.0 EXT=xz install: - sudo apt-get -qqy build-dep libvirt -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: Fix virStorageBackendIQNFound() to work on FreeBSD
On 07/25/2018 07:17 AM, Michal Privoznik wrote: > On 07/25/2018 11:30 AM, Andrea Bolognani wrote: >> Despite being standardized in POSIX.1-2008, the 'm' >> sscanf() modifier is currently not available on FreeBSD. >> >> Reimplement parsing without sscanf() to work around the >> issue. >> >> Signed-off-by: Andrea Bolognani >> --- >> src/util/viriscsi.c | 34 -- >> 1 file changed, 28 insertions(+), 6 deletions(-) > > Le sigh. So apparently we can't take 10 years old standard for granted > and have to use this ugly (as in not obvious) style of parsing. One > possibility is to go to BSD guys and ask them to comply with POSIX, but > that is very likely to fail. So as much as I hate to write this, > > ACK > > Michal Should have gone with the overly intrusive virStringSplit[Count] option like I suggested during review ;-) [which is at least partially open coded here, but I digress]. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 37/40] util: kmod: use VIR_AUTOFREE instead of VIR_FREE for scalar types
On Sat, Jul 21, 2018 at 05:37:09PM +0530, Sukrit Bhatnagar wrote: > By making use of GNU C's cleanup attribute handled by the > VIR_AUTOFREE macro for declaring scalar variables, majority > of the VIR_FREE calls 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 40/40] util: lease: use VIR_AUTOPTR for aggregate types
On Sat, Jul 21, 2018 at 05:37:12PM +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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 39/40] util: lease: use VIR_AUTOFREE instead of VIR_FREE for scalar types
On Sat, Jul 21, 2018 at 05:37:11PM +0530, Sukrit Bhatnagar wrote: > By making use of GNU C's cleanup attribute handled by the > VIR_AUTOFREE macro for declaring scalar variables, majority > of the VIR_FREE calls 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 38/40] util: kmod: use VIR_AUTOPTR for aggregate types
On Sat, Jul 21, 2018 at 05:37:10PM +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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx storage: Fix typo lsilogic -> lsiLogic
On 07/24/2018 02:15 AM, Marcos Paulo de Souza wrote: > Commit 77298458d027db4d3e082213355e2d792f65158d changed the esx storage > adapter from busLogic to lsilogic, introducing a typo. Changing it back > to lsiLogic (with capital L) solves the issue. With this change, libvirt can > now > create volumes in ESX again. > > Thanks to Jaroslav Suchanek who figured out what was the issue in the > first place. > > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1571759 > Signed-off-by: Marcos Paulo de Souza > --- > src/esx/esx_storage_backend_vmfs.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/esx/esx_storage_backend_vmfs.c > b/src/esx/esx_storage_backend_vmfs.c > index 630a6aa8c9..bb2de4b69f 100644 > --- a/src/esx/esx_storage_backend_vmfs.c > +++ b/src/esx/esx_storage_backend_vmfs.c > @@ -967,9 +967,9 @@ esxStorageVolCreateXML(virStoragePoolPtr pool, > /* > * FIXME: The adapter type is a required parameter, but there is no > * way to let the user specify it in the volume XML config. > Therefore, > - * default to 'lsilogic' here. > + * default to 'lsiLogic' here. > */ > -virtualDiskSpec->adapterType = (char *)"lsilogic"; > +virtualDiskSpec->adapterType = (char *)"lsiLogic"; > > virtualDiskSpec->capacityKb->value = >VIR_DIV_UP(def->target.capacity, 1024); /* Scale from byte to > kilobyte */ > Ooops. Yes. ACKed and pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 36/40] util: iscsi: use VIR_AUTOPTR for aggregate types
On Sat, Jul 21, 2018 at 05:37:08PM +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/viriscsi.c | 68 > + > 1 file changed, 22 insertions(+), 46 deletions(-) > > diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c > index 13fd02c..a3367bc 100644 > --- a/src/util/viriscsi.c > +++ b/src/util/viriscsi.c > @@ -83,7 +83,7 @@ virISCSIGetSession(const char *devpath, > VIR_AUTOFREE(char *) error = NULL; > int exitstatus = 0; > > -virCommandPtr cmd = virCommandNewArgList(ISCSIADM, "--mode", > +VIR_AUTOPTR(virCommand) cmd = virCommandNewArgList(ISCSIADM, "--mode", > "session", NULL); > virCommandSetErrorBuffer(cmd, ); > > @@ -93,15 +93,13 @@ virISCSIGetSession(const char *devpath, > vars, > virISCSIExtractSession, > , NULL, ) < 0) > -goto cleanup; > +return cbdata.session; I'd prefer you returned NULL explicitly to avoid any confusion. With that: Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 35/40] util: iscsi: use VIR_AUTOFREE instead of VIR_FREE for scalar types
On Sat, Jul 21, 2018 at 05:37:07PM +0530, Sukrit Bhatnagar wrote: > By making use of GNU C's cleanup attribute handled by the > VIR_AUTOFREE macro for declaring scalar variables, majority > of the VIR_FREE calls 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] IGNORE. Testing patchew.org syntax-check reporting
Hi, This series was run against 'syntax-check' test by patchew.org, which failed, please find the details below: Type: series Message-id: 20180725110439.27363-1-berra...@redhat.com Subject: [libvirt] [PATCH] IGNORE. Testing patchew.org syntax-check reporting === TEST SCRIPT BEGIN === #!/bin/bash # Testing script will be invoked under the git checkout with # HEAD pointing to a commit that has the patches applied on top of "base" # branch time bash -c './autogen.sh && make syntax-check' === TEST SCRIPT END === Updating bcb55ab053bc79561b55d0394490f4b64e0f2d01 >From https://github.com/patchew-project/libvirt * [new tag] patchew/20180725110439.27363-1-berra...@redhat.com -> patchew/20180725110439.27363-1-berra...@redhat.com Switched to a new branch 'test' bc7ccf39aa IGNORE. Testing patchew.org syntax-check reporting === OUTPUT BEGIN === Updating submodules... Submodule 'gnulib' (https://git.savannah.gnu.org/git/gnulib.git/) registered for path '.gnulib' Submodule 'keycodemapdb' (https://gitlab.com/keycodemap/keycodemapdb.git) registered for path 'src/keycodemapdb' Cloning into '/var/tmp/patchew-tester-tmp-y8h7scx3/src/.gnulib'... Cloning into '/var/tmp/patchew-tester-tmp-y8h7scx3/src/src/keycodemapdb'... Submodule path '.gnulib': checked out '68df637b5f1b5c10370f6981d2a43a5cf74368df' Submodule path 'src/keycodemapdb': checked out '16e5b0787687d8904dad2c026107409eb9bfcb95' Running bootstrap... ./bootstrap: Bootstrapping from checked-out libvirt sources... ./bootstrap: consider installing git-merge-changelog from gnulib ./bootstrap: getting gnulib files... running: libtoolize --install --copy libtoolize: putting auxiliary files in AC_CONFIG_AUX_DIR, 'build-aux'. libtoolize: copying file 'build-aux/config.guess' libtoolize: copying file 'build-aux/config.sub' libtoolize: copying file 'build-aux/install-sh' libtoolize: copying file 'build-aux/ltmain.sh' libtoolize: putting macros in AC_CONFIG_MACRO_DIRS, 'm4'. libtoolize: copying file 'm4/libtool.m4' libtoolize: copying file 'm4/ltoptions.m4' libtoolize: copying file 'm4/ltsugar.m4' libtoolize: copying file 'm4/ltversion.m4' libtoolize: copying file 'm4/lt~obsolete.m4' ./bootstrap: .gnulib/gnulib-tool--no-changelog --aux-dir=build-aux --doc-base=doc --lib=libgnu --m4-base=m4/ --source-base=gnulib/lib/ --tests-base=gnulib/tests --local-dir=gnulib/local--lgpl=2 --with-tests --makefile-name=gnulib.mk --avoid=pt_chown --avoid=lock-tests --libtool --import ... Module list with included dependencies (indented): absolute-header accept accept-tests alloca alloca-opt alloca-opt-tests allocator areadlink areadlink-tests arpa_inet arpa_inet-tests assure autobuild base64 base64-tests binary-io binary-io-tests bind bind-tests bitrotate bitrotate-tests btowc btowc-tests builtin-expect byteswap byteswap-tests c-ctype c-ctype-tests c-strcase c-strcase-tests c-strcasestr c-strcasestr-tests calloc-posix canonicalize-lgpl canonicalize-lgpl-tests careadlinkat chown chown-tests clock-time cloexec cloexec-tests close close-tests configmake connect connect-tests count-leading-zeros count-leading-zeros-tests count-one-bits count-one-bits-tests ctype ctype-tests dirname-lgpl dosname double-slash-root dup dup-tests dup2 dup2-tests environ environ-tests errno errno-tests error execinfo exitfail extensions extern-inline fatal-signal fclose fclose-tests fcntl fcntl-h fcntl-h-tests fcntl-tests fd-hook fdatasync fdatasync-tests fdopen fdopen-tests fflush fflush-tests ffs ffs-tests ffsl ffsl-tests fgetc-tests filename flexmember float float-tests fnmatch fnmatch-tests fpieee fpucw fpurge fpurge-tests fputc-tests fread-tests freading freading-tests fseek fseek-tests fseeko fseeko-tests fstat fstat-tests fsync fsync-tests ftell ftell-tests ftello ftello-tests ftruncate ftruncate-tests func func-tests fwrite-tests getaddrinfo getaddrinfo-tests getcwd-lgpl getcwd-lgpl-tests getdelim getdelim-tests getdtablesize getdtablesize-tests getgroups getgroups-tests gethostname gethostname-tests getline getline-tests getopt-posix getopt-posix-tests getpagesize getpass getpeername getpeername-tests getprogname getprogname-tests getsockname getsockname-tests getsockopt getsockopt-tests gettext-h gettimeofday gettimeofday-tests getugroups gitlog-to-changelog gnumakefile grantpt grantpt-tests hard-locale havelib host-cpu-c-abi hostent ignore-value ignore-value-tests include_next inet_ntop inet_ntop-tests
[libvirt] [dbus PATCH] tests: Fix sha-bang for test_interface.py
We need to call python3 through /usr/bin/env so that the script will work on platforms, such as FreeBSD, where the interpreter is installed under a different path. Signed-off-by: Andrea Bolognani --- Pushed under the build-breaker rule. tests/test_interface.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_interface.py b/tests/test_interface.py index 9503eef..3b8b344 100755 --- a/tests/test_interface.py +++ b/tests/test_interface.py @@ -1,4 +1,4 @@ -#!/usr/bin/python3 +#!/usr/bin/env python3 import dbus import libvirttest -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] news: Add --alias to virsh attach-disk and attach-interface
On 07/23/2018 04:12 PM, Han Han wrote: > Signed-off-by: Han Han > --- > docs/news.xml | 9 + > 1 file changed, 9 insertions(+) Tweaked the sentences a bit, ACKed and pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 34/40] util: iptables: use VIR_AUTOFREE instead of VIR_FREE for scalar types
On Sat, Jul 21, 2018 at 05:37:06PM +0530, Sukrit Bhatnagar wrote: > By making use of GNU C's cleanup attribute handled by the > VIR_AUTOFREE macro for declaring scalar variables, majority > of the VIR_FREE calls 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 33/40] util: hostmem: use VIR_AUTOFREE instead of VIR_FREE for scalar types
On Sat, Jul 21, 2018 at 05:37:05PM +0530, Sukrit Bhatnagar wrote: > By making use of GNU C's cleanup attribute handled by the > VIR_AUTOFREE macro for declaring scalar variables, majority > of the VIR_FREE calls 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 32/40] util: hostdev: use VIR_AUTOPTR for aggregate types
On Sat, Jul 21, 2018 at 05:37:04PM +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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 31/40] util: hostdev: use VIR_AUTOFREE instead of VIR_FREE for scalar types
On Sat, Jul 21, 2018 at 05:37:03PM +0530, Sukrit Bhatnagar wrote: > By making use of GNU C's cleanup attribute handled by the > VIR_AUTOFREE macro for declaring scalar variables, majority > of the VIR_FREE calls 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: Fix virStorageBackendIQNFound() to work on FreeBSD
On 07/25/2018 11:30 AM, Andrea Bolognani wrote: > Despite being standardized in POSIX.1-2008, the 'm' > sscanf() modifier is currently not available on FreeBSD. > > Reimplement parsing without sscanf() to work around the > issue. > > Signed-off-by: Andrea Bolognani > --- > src/util/viriscsi.c | 34 -- > 1 file changed, 28 insertions(+), 6 deletions(-) Le sigh. So apparently we can't take 10 years old standard for granted and have to use this ugly (as in not obvious) style of parsing. One possibility is to go to BSD guys and ask them to comply with POSIX, but that is very likely to fail. So as much as I hate to write this, ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 22/40] util: netdevvlan: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
On Sat, Jul 21, 2018 at 05:36:54PM +0530, Sukrit Bhatnagar wrote: > Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in > src/util/viralloc.h, define a new wrapper around an existing > cleanup function which will be called when a variable declared > with VIR_AUTOPTR macro goes out of scope. Also, drop the redundant > viralloc.h include, since that has moved from the source module into > the header. > > When a variable of type virNetDevVlanPtr is declared using > VIR_AUTOPTR, the function virNetDevVlanFree will be run > automatically on it when it goes out of scope. > > Signed-off-by: Sukrit Bhatnagar > --- This isn't used until patch 32, so it should be moved before patch 31 handling hostdevs. Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] IGNORE. Testing patchew.org syntax-check reporting
Signed-off-by: Daniel P. Berrangé --- src/libvirt.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libvirt.c b/src/libvirt.c index aeb8804714..ed4ec062e7 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -21,8 +21,6 @@ * Daniel Veillard */ -#include - #include #include #include -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 3/3] qemu: Use the correct vm def on cold attach
On 07/16/2018 11:14 PM, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1559867
>
> When attaching a device to the domain we need to be sure
> to use the correct domain definition (vm->def or vm->newDef)
> when calling virDomainDeviceDefParse because the post parse
> processing algorithms that may assign an address for the
> device will use whatever domain definition was passed in.
>
> Additionally, some devices (SCSI hostdev and SCSI disk) use
> algorithms that rely on knowing what already exists of the
> other type when generating the new device's address. Using
> the wrong VM definition could result in duplicated addresses.
>
> In the case of the bz, two hostdev's with no domain address
> provided were added to the running domain's config only.
> However, the parsing algorithm used the live domain in
> order to figure out the host device address resulting in
> the same address being used and a subsequent start failing
> due to duplicate address.
>
> Fix this by separating the checks/code into CONFIG and LIVE
> processing using the correct definition for each block and
> performing cleanup for both options as necessary.
>
> Signed-off-by: John Ferlan
> ---
> src/qemu/qemu_driver.c | 52 +++---
> 1 file changed, 23 insertions(+), 29 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 7d519c0714..3dbd5c62d2 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -8473,7 +8473,8 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
> {
> virDomainDefPtr vmdef = NULL;
> virQEMUDriverConfigPtr cfg = NULL;
> -virDomainDeviceDefPtr dev = NULL, dev_copy = NULL;
> +virDomainDeviceDefPtr devConf = NULL;
> +virDomainDeviceDefPtr devLive = NULL;
> int ret = -1;
> virCapsPtr caps = NULL;
> unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
> @@ -8487,49 +8488,43 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr
> vm,
> if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
> goto cleanup;
>
> -dev = dev_copy = virDomainDeviceDefParse(xml, vm->def,
> - caps, driver->xmlopt,
> - parse_flags);
> -if (dev == NULL)
> -goto cleanup;
> -
> -if (virDomainDeviceValidateAliasForHotplug(vm, dev, flags) < 0)
> -goto cleanup;
> -
> -if (flags & VIR_DOMAIN_AFFECT_CONFIG &&
> -flags & VIR_DOMAIN_AFFECT_LIVE) {
> -/* If we are affecting both CONFIG and LIVE
> - * create a deep copy of device as adding
> - * to CONFIG takes one instance.
> - */
> -dev_copy = virDomainDeviceDefCopy(dev, vm->def, caps,
> driver->xmlopt);
> -if (!dev_copy)
> -goto cleanup;
> -}
> -
> +/* The config and live post processing address auto-generation algorithms
> + * rely on the correct vm->def or vm->newDef being passed, so call the
> + * device parse based on which definition is in use */
> if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
> -/* Make a copy for updated domain. */
> vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt);
> if (!vmdef)
> goto cleanup;
>
> -if (virDomainDefCompatibleDevice(vmdef, dev, NULL,
> +if (!(devConf = virDomainDeviceDefParse(xml, vmdef, caps,
> +driver->xmlopt,
> parse_flags)))
> +goto cleanup;
> +
> +if (virDomainDefCompatibleDevice(vmdef, devConf, NULL,
> VIR_DOMAIN_DEVICE_ACTION_ATTACH,
> false) < 0)
> goto cleanup;
> -if ((ret = qemuDomainAttachDeviceConfig(vmdef, dev, caps,
> +
> +if ((ret = qemuDomainAttachDeviceConfig(vmdef, devConf, caps,
> parse_flags,
> driver->xmlopt)) < 0)
> goto cleanup;
> }
>
> if (flags & VIR_DOMAIN_AFFECT_LIVE) {
> -if (virDomainDefCompatibleDevice(vm->def, dev_copy, NULL,
> +if (!(devLive = virDomainDeviceDefParse(xml, vm->def, caps,
> +driver->xmlopt,
> parse_flags)))
> +goto cleanup;
> +
> +if (virDomainDeviceValidateAliasForHotplug(vm, devLive, flags) < 0)
> +goto cleanup;
> +
> +if (virDomainDefCompatibleDevice(vm->def, devLive, NULL,
> VIR_DOMAIN_DEVICE_ACTION_ATTACH,
> true) < 0)
> goto cleanup;
>
> -if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, driver)) < 0)
> +if ((ret = qemuDomainAttachDeviceLive(vm, devLive, driver)) < 0)
> goto cleanup;
> /*
> * update domain status
Re: [libvirt] [PATCH] util: Fix virStorageBackendIQNFound() to work on FreeBSD
On Wed, 2018-07-25 at 02:49 -0700, no-re...@patchew.org wrote: > Hi, > > This series was run against 'syntax-check' test by patchew.org, > which failed, please find the details below: [...] > src/util/viriscsi.c:152:int i; > maint.mk: use size_t, not int/unsigned int for loop vars i, j, k > make: *** [cfg.mk:573: sc_prohibit_int_ijk] Error 1 My bad. Consider the diff below squashed in. diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c index 96934df948..653b4fd932 100644 --- a/src/util/viriscsi.c +++ b/src/util/viriscsi.c @@ -149,7 +149,7 @@ virStorageBackendIQNFound(const char *initiatoriqn, char *current = line; char *newline; char *next; -int i; +size_t i; if (!(newline = strchr(line, '\n'))) break; -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 30/40] util: scsivhost: use VIR_AUTOPTR for aggregate types
On Sat, Jul 21, 2018 at 05:37:02PM +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/virscsivhost.c | 7 ++-
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/src/util/virscsivhost.c b/src/util/virscsivhost.c
> index ef216b3..3cd421e 100644
> --- a/src/util/virscsivhost.c
> +++ b/src/util/virscsivhost.c
> @@ -109,8 +109,7 @@ void
> virSCSIVHostDeviceListDel(virSCSIVHostDeviceListPtr list,
>virSCSIVHostDevicePtr dev)
> {
> -virSCSIVHostDevicePtr tmp = virSCSIVHostDeviceListSteal(list, dev);
> -virSCSIVHostDeviceFree(tmp);
> +VIR_AUTOPTR(virSCSIVHostDevice) tmp = virSCSIVHostDeviceListSteal(list,
> dev);
> }
>
>
> @@ -271,13 +270,11 @@ virSCSIVHostDeviceNew(const char *name)
>
> VIR_DEBUG("%s: initialized", dev->name);
>
> - cleanup:
> return dev;
>
> error:
> virSCSIVHostDeviceFree(dev);
> -dev = NULL;
> -goto cleanup;
> +return NULL;
If you declared @dev as VIR_AUTOPTR along with a non-VIR_AUTOPTR @ret and then
used VIR_STEAL_PTR and returned @ret instead, then you could get rid of all the
goto labels in ^this function.
With that tiny tweak:
Reviewed-by: Erik Skultety
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 29/40] util: scsivhost: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
On Sat, Jul 21, 2018 at 05:37:01PM +0530, Sukrit Bhatnagar wrote: > Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in > src/util/viralloc.h, define a new wrapper around an existing > cleanup function which will be called when a variable declared > with VIR_AUTOPTR macro goes out of scope. Also, drop the redundant > viralloc.h include, since that has moved from the source module into > the header. > > When a variable of type virSCSIVHostDevicePtr is declared using > VIR_AUTOPTR, the function virSCSIVHostDeviceFree will be run > automatically on it when it goes out of scope. > > 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 v1 28/40] util: scsi: use VIR_AUTOPTR for aggregate types
On Sat, Jul 21, 2018 at 05:37:00PM +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/virscsi.c | 38 --
> 1 file changed, 16 insertions(+), 22 deletions(-)
>
> diff --git a/src/util/virscsi.c b/src/util/virscsi.c
> index ba0a688..abe699b 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,44 @@ 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);
> +dev = NULL;
I'd suggest using VIR_STEAL_PTR instead.
> return ret;
> }
>
> @@ -281,21 +280,17 @@ virSCSIDeviceSetUsedBy(virSCSIDevicePtr dev,
> const char *drvname,
> const char *domname)
> {
> -virUsedByInfoPtr copy;
> +VIR_AUTOPTR(virUsedByInfo) copy = NULL;
> if (VIR_ALLOC(copy) < 0)
> return -1;
> if (VIR_STRDUP(copy->drvname, drvname) < 0 ||
> VIR_STRDUP(copy->domname, domname) < 0)
> -goto cleanup;
> +return -1;
>
> if (VIR_APPEND_ELEMENT(dev->used_by, dev->n_used_by, copy) < 0)
> -goto cleanup;
> +return -1;
>
> return 0;
> -
> - cleanup:
> -virSCSIDeviceUsedByInfoFree(copy);
> -return -1;
> }
>
> bool
> @@ -442,7 +437,6 @@ virSCSIDeviceListDel(virSCSIDeviceListPtr list,
> const char *drvname,
> const char *domname)
> {
> -virSCSIDevicePtr tmp = NULL;
> size_t i;
>
> for (i = 0; i < dev->n_used_by; i++) {
> @@ -452,8 +446,8 @@ virSCSIDeviceListDel(virSCSIDeviceListPtr list,
> virSCSIDeviceUsedByInfoFree(dev->used_by[i]);
> VIR_DELETE_ELEMENT(dev->used_by, i, dev->n_used_by);
> } else {
> -tmp = virSCSIDeviceListSteal(list, dev);
> -virSCSIDeviceFree(tmp);
> +VIR_AUTOPTR(virSCSIDevice) tmp =
> +virSCSIDeviceListSteal(list, dev);
No need to save the lines that much, simply declare the variable on a separate
line:
VIR_AUTOPTR(virSCSIDevice) tmp = NULL;
tmp = virSCSIDeviceListSteal(list, dev);
With that:
Reviewed-by: Erik Skultety
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 27/40] util: scsi: use VIR_AUTOFREE instead of VIR_FREE for scalar types
On Sat, Jul 21, 2018 at 05:36:59PM +0530, Sukrit Bhatnagar wrote: > By making use of GNU C's cleanup attribute handled by the > VIR_AUTOFREE macro for declaring scalar variables, majority > of the VIR_FREE calls 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 26/40] util: scsi: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
On Sat, Jul 21, 2018 at 05:36:58PM +0530, Sukrit Bhatnagar wrote: > Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in > src/util/viralloc.h, define a new wrapper around an existing > cleanup function which will be called when a variable declared > with VIR_AUTOPTR macro goes out of scope. Also, drop the redundant > viralloc.h include, since that has moved from the source module into > the header. > > When variables of type virSCSIDevicePtr and virUsedByInfoPtr > are declared using VIR_AUTOPTR, the functions virSCSIDeviceFree > and virSCSIDeviceUsedByInfoFree, respectively, will be run > automatically on them when they go out of scope. > > This commit also adds an intermediate typedef for virCgroup type > for use with the cleanup macros by renaming the struct in > src/util/vircgrouppriv.h from virCgroup to _virCgroup. You need to tweak ^this copy-paste to match the module ;). Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 08/40] util: cgroup: define cleanup function using VIR_DEFINE_AUTOPTR_FUNC
On Sat, Jul 21, 2018 at 05:36:40PM +0530, Sukrit Bhatnagar wrote: > Using the new VIR_DEFINE_AUTOPTR_FUNC macro defined in > src/util/viralloc.h, define a new wrapper around an existing > cleanup function which will be called when a variable declared > with VIR_AUTOPTR macro goes out of scope. Also, drop the redundant > viralloc.h include, since that has moved from the source module into > the header. > > When a variable of type virCgroupPtr is declared using > VIR_AUTOPTR, the function virCgroupFree will be run > automatically on it when it goes out of scope. > > This commit also adds an intermediate typedef for virCgroup type > for use with the cleanup macros by renaming the struct in > src/util/vircgrouppriv.h from virCgroup to _virCgroup. How about: ... typedef to virCgroup type in order to make use of the VIR_DEFINE_AUTOPTR_FUNC macro. Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH] guests: Make mappings more future proof
On Wed, Jun 13, 2018 at 03:01:48PM +0200, Andrea Bolognani wrote: > We can reasonably expect the next major release of CentOS > to be somewhat close to current Fedora releases in terms of > packaging, and to ditch the soon-to-be-unsupported Python 2 > in favor of Python 3. > > Rewrite some of the mappings based on these expectations. > > Signed-off-by: Andrea Bolognani > --- > guests/vars/mappings.yml | 18 +- > 1 file changed, 9 insertions(+), 9 deletions(-) Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 25/40] util: usb: use VIR_AUTOPTR for aggregate types
On Sat, Jul 21, 2018 at 05:36:57PM +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 | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/src/util/virusb.c b/src/util/virusb.c
> index c14683f..cfeac51 100644
> --- a/src/util/virusb.c
> +++ b/src/util/virusb.c
> @@ -508,8 +508,7 @@ void
> virUSBDeviceListDel(virUSBDeviceListPtr list,
> virUSBDevicePtr dev)
> {
> -virUSBDevicePtr ret = virUSBDeviceListSteal(list, dev);
> -virUSBDeviceFree(ret);
> +VIR_AUTOPTR(virUSBDevice) ret = virUSBDeviceListSteal(list, dev);
> }
>
Technically, there's also a virUSBDevicePtr instance in virUSBDeviceSearch that
could be converted to VIR_AUTOPTR, but virUSBDeviceListAdd would have to take a
double pointer to @dev instead of a single pointer. A bit more background
info - the current issue is that virUSBDeviceListAdd calls our
VIR_APPEND_ELEMENT helper which does clear the original pointer which we could
utilize here, but not while passing a single pointer.
Not a deal breaker, though, it's just a nice to have, since you're already
working in this area, because I don't suppose we'd make such a change any time
soon after your assignment is over.
(regardless)
Reviewed-by: Erik Skultety
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] util: Fix virStorageBackendIQNFound() to work on FreeBSD
Despite being standardized in POSIX.1-2008, the 'm'
sscanf() modifier is currently not available on FreeBSD.
Reimplement parsing without sscanf() to work around the
issue.
Signed-off-by: Andrea Bolognani
---
src/util/viriscsi.c | 34 --
1 file changed, 28 insertions(+), 6 deletions(-)
diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c
index e2c80acaaa..96934df948 100644
--- a/src/util/viriscsi.c
+++ b/src/util/viriscsi.c
@@ -146,8 +146,10 @@ virStorageBackendIQNFound(const char *initiatoriqn,
line = outbuf;
while (line && *line) {
+char *current = line;
char *newline;
-int num;
+char *next;
+int i;
if (!(newline = strchr(line, '\n')))
break;
@@ -156,15 +158,29 @@ virStorageBackendIQNFound(const char *initiatoriqn,
VIR_FREE(iface);
VIR_FREE(iqn);
-num = sscanf(line, "%ms %*[^,],%*[^,],%*[^,],%*[^,],%ms", ,
);
-if (num != 2) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("malformed output of %s: %s"),
- ISCSIADM, line);
+/* Find the first space, copy everything up to that point into
+ * iface and move past it to continue processing */
+if (!(next = strchr(current, ' ')))
+goto error;
+
+if (VIR_STRNDUP(iface, current, (next - current)) < 0)
goto cleanup;
+
+current = next + 1;
+
+/* There are five comma separated fields after iface and we only
+ * care about the last one, so we need to skip four commas and
+ * copy whatever's left into iqn */
+for (i = 0; i < 4; i++) {
+if (!(next = strchr(current, ',')))
+goto error;
+current = next + 1;
}
+if (VIR_STRDUP(iqn, current) < 0)
+goto cleanup;
+
if (STREQ(iqn, initiatoriqn)) {
VIR_STEAL_PTR(*ifacename, iface);
@@ -186,6 +202,12 @@ virStorageBackendIQNFound(const char *initiatoriqn,
VIR_FREE(outbuf);
virCommandFree(cmd);
return ret;
+
+ error:
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("malformed output of %s: %s"),
+ ISCSIADM, line);
+goto cleanup;
}
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 24/40] util: usb: use VIR_AUTOFREE instead of VIR_FREE for scalar types
On Sat, Jul 21, 2018 at 05:36:56PM +0530, Sukrit Bhatnagar wrote: > By making use of GNU C's cleanup attribute handled by the > VIR_AUTOFREE macro for declaring scalar variables, majority > of the VIR_FREE calls 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] vnc: remove deprecated TLS related features
Daniel P. Berrangé (2): doc: switch to modern syntax for VNC TLS setup vnc: remove support for deprecated tls, x509, x509verify options qemu-deprecated.texi | 20 -- qemu-doc.texi| 20 +++--- qemu-options.hx | 43 - ui/vnc.c | 91 4 files changed, 15 insertions(+), 159 deletions(-) -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] vnc: remove support for deprecated tls, x509, x509verify options
The 'tls-creds' option accepts the name of a TLS credentials
object. This replaced the usage of 'tls', 'x509' and 'x509verify'
options in 2.5.0. These deprecated options were grandfathered in
when the deprecation policy was introduded in 2.10.0, so can now
finally be removed.
Signed-off-by: Daniel P. Berrangé
---
qemu-deprecated.texi | 20 --
qemu-options.hx | 43 -
ui/vnc.c | 91
3 files changed, 154 deletions(-)
diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 9920a85adc..6b92cec48a 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -40,26 +40,6 @@ which is the default.
The ``-no-kvm'' argument is now a synonym for setting
``-machine accel=tcg''.
-@subsection -vnc tls (since 2.5.0)
-
-The ``-vnc tls'' argument is now a synonym for setting
-``-object tls-creds-anon,id=tls0'' combined with
-``-vnc tls-creds=tls0'
-
-@subsection -vnc x509 (since 2.5.0)
-
-The ``-vnc x509=/path/to/certs'' argument is now a
-synonym for setting
-``-object tls-creds-x509,dir=/path/to/certs,id=tls0,verify-peer=no''
-combined with ``-vnc tls-creds=tls0'
-
-@subsection -vnc x509verify (since 2.5.0)
-
-The ``-vnc x509verify=/path/to/certs'' argument is now a
-synonym for setting
-``-object tls-creds-x509,dir=/path/to/certs,id=tls0,verify-peer=yes''
-combined with ``-vnc tls-creds=tls0'
-
@subsection -tftp (since 2.6.0)
The ``-tftp /some/dir'' argument is replaced by either
diff --git a/qemu-options.hx b/qemu-options.hx
index b1bf0f485f..bf09c5b4e7 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1654,49 +1654,6 @@ will cause the VNC server socket to enable the VeNCrypt
auth
mechanism. The credentials should have been previously created
using the @option{-object tls-creds} argument.
-The @option{tls-creds} parameter obsoletes the @option{tls},
-@option{x509}, and @option{x509verify} options, and as such
-it is not permitted to set both new and old type options at
-the same time.
-
-@item tls
-
-Require that client use TLS when communicating with the VNC server. This
-uses anonymous TLS credentials so is susceptible to a man-in-the-middle
-attack. It is recommended that this option be combined with either the
-@option{x509} or @option{x509verify} options.
-
-This option is now deprecated in favor of using the @option{tls-creds}
-argument.
-
-@item x509=@var{/path/to/certificate/dir}
-
-Valid if @option{tls} is specified. Require that x509 credentials are used
-for negotiating the TLS session. The server will send its x509 certificate
-to the client. It is recommended that a password be set on the VNC server
-to provide authentication of the client when this is used. The path following
-this option specifies where the x509 certificates are to be loaded from.
-See the @ref{vnc_security} section for details on generating certificates.
-
-This option is now deprecated in favour of using the @option{tls-creds}
-argument.
-
-@item x509verify=@var{/path/to/certificate/dir}
-
-Valid if @option{tls} is specified. Require that x509 credentials are used
-for negotiating the TLS session. The server will send its x509 certificate
-to the client, and request that the client send its own x509 certificate.
-The server will validate the client's certificate against the CA certificate,
-and reject clients when validation fails. If the certificate authority is
-trusted, this is a sufficient authentication mechanism. You may still wish
-to set a password on the VNC server as a second authentication layer. The
-path following this option specifies where the x509 certificates are to
-be loaded from. See the @ref{vnc_security} section for details on generating
-certificates.
-
-This option is now deprecated in favour of using the @option{tls-creds}
-argument.
-
@item sasl
Require that the client use SASL to authenticate with the VNC server.
diff --git a/ui/vnc.c b/ui/vnc.c
index 359693238b..fd929b0957 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3344,10 +3344,6 @@ static QemuOptsList qemu_vnc_opts = {
},{
.name = "tls-creds",
.type = QEMU_OPT_STRING,
-},{
-/* Deprecated in favour of tls-creds */
-.name = "x509",
-.type = QEMU_OPT_STRING,
},{
.name = "share",
.type = QEMU_OPT_STRING,
@@ -3384,14 +3380,6 @@ static QemuOptsList qemu_vnc_opts = {
},{
.name = "sasl",
.type = QEMU_OPT_BOOL,
-},{
-/* Deprecated in favour of tls-creds */
-.name = "tls",
-.type = QEMU_OPT_BOOL,
-},{
-/* Deprecated in favour of tls-creds */
-.name = "x509verify",
-.type = QEMU_OPT_STRING,
},{
.name = "acl",
.type = QEMU_OPT_BOOL,
@@ -3519,51 +3507,6 @@ vnc_display_setup_auth(int *auth,
}
-/*
- * Handle back compat with old CLI syntax by creating some
- * suitable
[libvirt] [PATCH 1/2] doc: switch to modern syntax for VNC TLS setup
The use of 'tls', 'x509' and 'x509verify' properties is the deprecated
backcompat syntax, replaced by use of TLS creds objects.
Signed-off-by: Daniel P. Berrangé
---
qemu-doc.texi | 20 +++-
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/qemu-doc.texi b/qemu-doc.texi
index abfd2db546..080548f79c 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -1097,7 +1097,9 @@ support provides a secure session, but no authentication.
This allows any
client to connect, and provides an encrypted session.
@example
-qemu-system-i386 [...OPTIONS...] -vnc :1,tls,x509=/etc/pki/qemu -monitor stdio
+qemu-system-i386 [...OPTIONS...] \
+ -object
tls-creds-x509,id=tls0,dir=/etc/pki/qemu,endpoint=server,verify-peer=no \
+ -vnc :1,tls-creds=tls0 -monitor stdio
@end example
In the above example @code{/etc/pki/qemu} should contain at least three files,
@@ -1112,10 +1114,14 @@ only be readable by the user owning it.
Certificates can also provide a means to authenticate the client connecting.
The server will request that the client provide a certificate, which it will
then validate against the CA certificate. This is a good choice if deploying
-in an environment with a private internal certificate authority.
+in an environment with a private internal certificate authority. It uses the
+same syntax as previously, but with @code{verify-peer} set to @code{yes}
+instead.
@example
-qemu-system-i386 [...OPTIONS...] -vnc :1,tls,x509verify=/etc/pki/qemu -monitor
stdio
+qemu-system-i386 [...OPTIONS...] \
+ -object
tls-creds-x509,id=tls0,dir=/etc/pki/qemu,endpoint=server,verify-peer=yes \
+ -vnc :1,tls-creds=tls0 -monitor stdio
@end example
@@ -1126,7 +1132,9 @@ Finally, the previous method can be combined with VNC
password authentication
to provide two layers of authentication for clients.
@example
-qemu-system-i386 [...OPTIONS...] -vnc :1,password,tls,x509verify=/etc/pki/qemu
-monitor stdio
+qemu-system-i386 [...OPTIONS...] \
+ -object
tls-creds-x509,id=tls0,dir=/etc/pki/qemu,endpoint=server,verify-peer=yes \
+ -vnc :1,tls-creds=tls0,password -monitor stdio
(qemu) change vnc password
Password:
(qemu)
@@ -1163,7 +1171,9 @@ credentials. This can be enabled, by combining the 'sasl'
option
with the aforementioned TLS + x509 options:
@example
-qemu-system-i386 [...OPTIONS...] -vnc :1,tls,x509,sasl -monitor stdio
+qemu-system-i386 [...OPTIONS...] \
+ -object
tls-creds-x509,id=tls0,dir=/etc/pki/qemu,endpoint=server,verify-peer=yes \
+ -vnc :1,tls-creds=tls0,sasl -monitor stdio
@end example
@node vnc_setup_sasl
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
