Re: [libvirt] [PATCH v4 3/4] qemu: block: store the delete flag in libvirtd's status XML
On Tue, Dec 10, 2019 at 17:25:40 +0100, Pavel Mores wrote: > Since blockcommit is asynchronous, libvirtd can be restarted while the > operation runs. To ensure the information necessary to finish up the job > is not lost, serialisation to and deserialisation from the status XML is > added. > > To unittest this, the new element was only added to the active commit test, > the non-active commit test doesn't have the new element so as to test its > absence. > > Signed-off-by: Pavel Mores > --- > src/qemu/qemu_domain.c | 4 > tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml | 1 + > 2 files changed, 5 insertions(+) Reviewed-by: Peter Krempa -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 2/4] qemu: block: use the delete flag to delete snapshot images if requested
On Tue, Dec 10, 2019 at 17:25:39 +0100, Pavel Mores wrote: > When blockcommit finishes successfully, one of the > qemuBlockJobProcessEventCompletedCommit() and > qemuBlockJobProcessEventCompletedActiveCommit() event handlers is called. > This is where the delete flag (stored in qemuBlockJobCommitData since the > previous commit) can actually be used to delete the committed snapshot > images if requested. > > We use virFileRemove() instead of a simple unlink() to cover the case where > the image to be removed is on an NFS volume. > > Signed-off-by: Pavel Mores > --- > src/qemu/qemu_blockjob.c | 46 > 1 file changed, 46 insertions(+) Reviewed-by: Peter Krempa -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 1/4] qemu: block: propagate the delete flag to where it can actually be used
On Tue, Dec 10, 2019 at 17:25:38 +0100, Pavel Mores wrote: > Propagate the delete flag from qemuDomainBlockCommit() (which was just > ignoring it until now) to qemuBlockJobDiskNewCommit() where it can be > stored in the qemuBlockJobCommitData structure which holds information > necessary to finish the job asynchronously. > > In the actual qemuBlockJobDiskNewCommit() in this commit, we temporarily > pass a literal 'false' to preserve the current behaviour until the whole > implementation of the feature is in place. > > Signed-off-by: Pavel Mores > --- > src/qemu/qemu_blockjob.c | 4 +++- > src/qemu/qemu_blockjob.h | 4 +++- > src/qemu/qemu_driver.c | 2 +- > 3 files changed, 7 insertions(+), 3 deletions(-) Reviewed-by: Peter Krempa -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 4/4] qemu: block: enable the snapshot image deletion feature
On Tue, Dec 10, 2019 at 17:25:41 +0100, Pavel Mores wrote:
> With all plumbing in place, we can now enable the new functionality.
>
> Signed-off-by: Pavel Mores
> ---
> src/qemu/qemu_driver.c | 12 ++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 9c07b6b393..0cbc746c22 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -18544,10 +18544,10 @@ qemuDomainBlockCommit(virDomainPtr dom,
> bool persistjob = false;
> bool blockdev = false;
>
> -/* XXX Add support for COMMIT_DELETE */
> virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW |
>VIR_DOMAIN_BLOCK_COMMIT_ACTIVE |
>VIR_DOMAIN_BLOCK_COMMIT_RELATIVE |
> + VIR_DOMAIN_BLOCK_COMMIT_DELETE |
>VIR_DOMAIN_BLOCK_COMMIT_BANDWIDTH_BYTES, -1);
>
> if (!(vm = qemuDomainObjFromDomain(dom)))
> @@ -18568,6 +18568,13 @@ qemuDomainBlockCommit(virDomainPtr dom,
>
> blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV);
>
> +if (!blockdev && (flags & VIR_DOMAIN_BLOCK_COMMIT_DELETE)) {
> +virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> + _("deleting committed images is only supported for
> VMs "
> + "with blockdev enabled"));
> +goto endjob;
> +}
make[1]: Leaving directory '/home/pipo/build/libvirt/gcc/src'
/home/pipo/libvirt/src/qemu/qemu_driver.c:18572:
virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
/home/pipo/libvirt/src/qemu/qemu_driver.c-18573-
_("deleting committed images is only supported for VMs "
/home/pipo/libvirt/src/qemu/qemu_driver.c-18574- "with
blockdev enabled"));
build-aux/syntax-check.mk: found diagnostic without %
make: *** [/home/pipo/libvirt/build-aux/syntax-check.mk:756:
sc_prohibit_diagnostic_without_format] Error 1
Please make sure you run 'make syntax-check' before submitting patches.
I'll fix this before pushing with with:
if (!blockdev && (flags & VIR_DOMAIN_BLOCK_COMMIT_DELETE)) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("deleting committed images is not supported by this
VM"));
goto endjob;
}
Reviewed-by: Peter Krempa
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 00/26] move qemucaps validations from qemu_command to qemu_domain
On Tue, Dec 10, 2019 at 19:45:01 -0300, Daniel Henrique Barboza wrote: > > > On 12/10/19 5:00 AM, Peter Krempa wrote: > > On Mon, Dec 09, 2019 at 20:15:05 -0300, Daniel Henrique Barboza wrote: > > > (series based on master commit 97cafa610ecf5) > > > > > > This work was proposed by Cole in [1]. This is Cole's reasoning for > > > it, copy/pasted from [1]: > > > > Nice work. Doing this was long overdue. My only suggestion is that after > > this we should move all validation into a separate file. qemu_domain was > > a code dumping place for a long time and since we now have a lot of > > common code moving it out would be benficial for cleaning up an making > > it more obvious. > > > Got it. We can create this new file and move the validations from > qemu_domain.c > to the new file, before resuming moving more code out of qemu_command.c. > That way > we avoid moving the code twice. Since you've got patches already it's okay to do it in two steps. It will be much appreciated if you move it to a new file, but it's not required for this series. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH 4/9] vfio-pci: register default dynamic-trap-bar-info region
On Wed, Dec 11, 2019 at 12:38:05AM +0800, Alex Williamson wrote: > On Tue, 10 Dec 2019 02:44:44 -0500 > Yan Zhao wrote: > > > On Tue, Dec 10, 2019 at 05:16:08AM +0800, Alex Williamson wrote: > > > On Mon, 9 Dec 2019 01:22:12 -0500 > > > Yan Zhao wrote: > > > > > > > On Fri, Dec 06, 2019 at 11:20:38PM +0800, Alex Williamson wrote: > > > > > On Fri, 6 Dec 2019 01:04:07 -0500 > > > > > Yan Zhao wrote: > > > > > > > > > > > On Fri, Dec 06, 2019 at 07:55:30AM +0800, Alex Williamson wrote: > > > > > > > On Wed, 4 Dec 2019 22:26:50 -0500 > > > > > > > Yan Zhao wrote: > > > > > > > > > > > > > > > Dynamic trap bar info region is a channel for QEMU and vendor > > > > > > > > driver to > > > > > > > > communicate dynamic trap info. It is of type > > > > > > > > VFIO_REGION_TYPE_DYNAMIC_TRAP_BAR_INFO and subtype > > > > > > > > VFIO_REGION_SUBTYPE_DYNAMIC_TRAP_BAR_INFO. > > > > > > > > > > > > > > > > This region has two fields: dt_fd and trap. > > > > > > > > When QEMU detects a device regions of this type, it will create > > > > > > > > an > > > > > > > > eventfd and write its eventfd id to dt_fd field. > > > > > > > > When vendor drivre signals this eventfd, QEMU reads trap field > > > > > > > > of this > > > > > > > > info region. > > > > > > > > - If trap is true, QEMU would search the device's PCI BAR > > > > > > > > regions and disable all the sparse mmaped subregions (if the > > > > > > > > sparse > > > > > > > > mmaped subregion is disablable). > > > > > > > > - If trap is false, QEMU would re-enable those subregions. > > > > > > > > > > > > > > > > A typical usage is > > > > > > > > 1. vendor driver first cuts its bar 0 into several sections, > > > > > > > > all in a > > > > > > > > sparse mmap array. So initally, all its bar 0 are passthroughed. > > > > > > > > 2. vendor driver specifys part of bar 0 sections to be > > > > > > > > disablable. > > > > > > > > 3. on migration starts, vendor driver signals dt_fd and set > > > > > > > > trap to true > > > > > > > > to notify QEMU disabling the bar 0 sections of disablable flags > > > > > > > > on. > > > > > > > > 4. QEMU disables those bar 0 section and hence let vendor > > > > > > > > driver be able > > > > > > > > to trap access of bar 0 registers and make dirty page tracking > > > > > > > > possible. > > > > > > > > 5. on migration failure, vendor driver signals dt_fd to QEMU > > > > > > > > again. > > > > > > > > QEMU reads trap field of this info region which is false and > > > > > > > > QEMU > > > > > > > > re-passthrough the whole bar 0 region. > > > > > > > > > > > > > > > > Vendor driver specifies whether it supports > > > > > > > > dynamic-trap-bar-info region > > > > > > > > through cap VFIO_PCI_DEVICE_CAP_DYNAMIC_TRAP_BAR in > > > > > > > > vfio_pci_mediate_ops->open(). > > > > > > > > > > > > > > > > If vfio-pci detects this cap, it will create a default > > > > > > > > dynamic_trap_bar_info region on behalf of vendor driver with > > > > > > > > region len=0 > > > > > > > > and region->ops=null. > > > > > > > > Vvendor driver should override this region's len, flags, rw, > > > > > > > > mmap in its > > > > > > > > vfio_pci_mediate_ops. > > > > > > > > > > > > > > TBH, I don't like this interface at all. Userspace doesn't pass > > > > > > > data > > > > > > > to the kernel via INFO ioctls. We have a SET_IRQS ioctl for > > > > > > > configuring user signaling with eventfds. I think we only need to > > > > > > > define an IRQ type that tells the user to re-evaluate the sparse > > > > > > > mmap > > > > > > > information for a region. The user would enumerate the device > > > > > > > IRQs via > > > > > > > GET_IRQ_INFO, find one of this type where the IRQ info would also > > > > > > > indicate which region(s) should be re-evaluated on signaling. > > > > > > > The user > > > > > > > would enable that signaling via SET_IRQS and simply re-evaluate > > > > > > > the > > > > > > ok. I'll try to switch to this way. Thanks for this suggestion. > > > > > > > > > > > > > sparse mmap capability for the associated regions when signaled. > > > > > > > > > > > > > > > > > > > Do you like the "disablable" flag of sparse mmap ? > > > > > > I think it's a lightweight way for user to switch mmap state of a > > > > > > whole region, > > > > > > otherwise going through a complete flow of GET_REGION_INFO and > > > > > > re-setup > > > > > > region might be too heavy. > > > > > > > > > > No, I don't like the disable-able flag. At what frequency do we > > > > > expect > > > > > regions to change? It seems like we'd only change when switching into > > > > > and out of the _SAVING state, which is rare. It seems easy for > > > > > userspace, at least QEMU, to drop the entire mmap configuration and > > > > > > > > > ok. I'll try this way. > > > > > > > > > re-read it. Another concern here is how do we synchronize the event? > > > > > Are we assuming that this event would
Re: [libvirt] [RFC PATCH 1/9] vfio/pci: introduce mediate ops to intercept vfio-pci ops
On Wed, Dec 11, 2019 at 12:58:24AM +0800, Alex Williamson wrote:
> On Mon, 9 Dec 2019 21:44:23 -0500
> Yan Zhao wrote:
>
> > > > > > Currently, yes, i40e has build dependency on vfio-pci.
> > > > > > It's like this, if i40e decides to support SRIOV and compiles in vf
> > > > > > related code who depends on vfio-pci, it will also have build
> > > > > > dependency
> > > > > > on vfio-pci. isn't it natural?
> > > > >
> > > > > No, this is not natural. There are certainly i40e VF use cases that
> > > > > have no interest in vfio and having dependencies between the two
> > > > > modules is unacceptable. I think you probably want to modularize the
> > > > > i40e vfio support code and then perhaps register a table in vfio-pci
> > > > > that the vfio-pci code can perform a module request when using a
> > > > > compatible device. Just and idea, there might be better options. I
> > > > > will not accept a solution that requires unloading the i40e driver in
> > > > > order to unload the vfio-pci driver. It's inconvenient with just one
> > > > > NIC driver, imagine how poorly that scales.
> > > > >
> > > > what about this way:
> > > > mediate driver registers a module notifier and every time when
> > > > vfio_pci is loaded, register to vfio_pci its mediate ops?
> > > > (Just like in below sample code)
> > > > This way vfio-pci is free to unload and this registering only gives
> > > > vfio-pci a name of what module to request.
> > > > After that,
> > > > in vfio_pci_open(), vfio-pci requests the mediate driver. (or puts
> > > > the mediate driver when mediate driver does not support mediating the
> > > > device)
> > > > in vfio_pci_release(), vfio-pci puts the mediate driver.
> > > >
> > > > static void register_mediate_ops(void)
> > > > {
> > > > int (*func)(struct vfio_pci_mediate_ops *ops) = NULL;
> > > >
> > > > func = symbol_get(vfio_pci_register_mediate_ops);
> > > >
> > > > if (func) {
> > > > func(_dt_ops);
> > > > symbol_put(vfio_pci_register_mediate_ops);
> > > > }
> > > > }
> > > >
> > > > static int igd_module_notify(struct notifier_block *self,
> > > > unsigned long val, void *data)
> > > > {
> > > > struct module *mod = data;
> > > > int ret = 0;
> > > >
> > > > switch (val) {
> > > > case MODULE_STATE_LIVE:
> > > > if (!strcmp(mod->name, "vfio_pci"))
> > > > register_mediate_ops();
> > > > break;
> > > > case MODULE_STATE_GOING:
> > > > break;
> > > > default:
> > > > break;
> > > > }
> > > > return ret;
> > > > }
> > > >
> > > > static struct notifier_block igd_module_nb = {
> > > > .notifier_call = igd_module_notify,
> > > > .priority = 0,
> > > > };
> > > >
> > > >
> > > >
> > > > static int __init igd_dt_init(void)
> > > > {
> > > > ...
> > > > register_mediate_ops();
> > > > register_module_notifier(_module_nb);
> > > > ...
> > > > return 0;
> > > > }
> > >
> > >
> > > No, this is bad. Please look at MODULE_ALIAS() and request_module() as
> > > used in the vfio-platform for loading reset driver modules. I think
> > > the correct approach is that vfio-pci should perform a request_module()
> > > based on the device being probed. Having the mediation provider
> > > listening for vfio-pci and registering itself regardless of whether we
> > > intend to use it assumes that we will want to use it and assumes that
> > > the mediation provider module is already loaded. We should be able to
> > > support demand loading of modules that may serve no other purpose than
> > > providing this mediation. Thanks,
> > hi Alex
> > Thanks for this message.
> > So is it good to create a separate module as mediation provider driver,
> > and alias its module name to "vfio-pci-mediate-vid-did".
> > Then when vfio-pci probes the device, it requests module of that name ?
>
> I think this would give us an option to have the mediator as a separate
> module, but not require it. Maybe rather than a request_module(),
> where if we follow the platform reset example we'd then expect the init
> code for the module to register into a list, we could do a
> symbol_request(). AIUI, this would give us a reference to the symbol
> if the module providing it is already loaded, and request a module
> (perhaps via an alias) if it's not already load. Thanks,
>
ok. got it!
Thank you :)
Yan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/17] docs: remove all use of POD for man pages in favour of RST
On 12/6/19 9:50 AM, Daniel P. Berrangé wrote: > As part of the goal to eliminate Perl from libvirt, this gets rid of the > use of POD format for man pages. There's nothing especially bad about > POD as a markup language compared to other lightweight markup languages > like RST or Markdown. It hasn't found widespread usage outside of the > Perl world though, and so switching from POD to RST brings a language > which likely has more familiarity to contributors. > > This also nicely aligns with our use of RST of web pages, and indeed > in this series things are setup so that all the man pages get published > on the main libvirt.org website. Over time this will hopefuly draw > search engines traffic to libvirt.org instead of random 3rd party > websites hosting various out of date copies of the man pages. > > Reviewing the individual RST files is likely a very unpleasant task, > especially the enourmous virsh man page. Most of the conversion work > was automated with pod2rst, followed by lots of editting to cleanup > its output. virsh had some further automated processing done to create > headers for each command. > > It is probably more useful to review the rendered man page output > and/or websites to see that it looks sane when read. > Reviewed-by: Cole Robinson With some tweaks: * There's a leftover pod2man in the spec file. * This conflicts with the virsh --tls-destination changes so don't forget to re-merge those * virt-host-validate patch needs this diff diff --git a/docs/Makefile.am b/docs/Makefile.am index e1f8f7646d..4027c2e26c 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -236,7 +236,7 @@ manpages_rst += \ $(NULL) endif ! WITH_LIBVIRTD if WITH_HOST_VALIDATE - manpages8_rst += manpages/virt-host-validate.rst + manpages1_rst += manpages/virt-host-validate.rst else ! WITH_HOST_VALIDATE manpages_rst += manpages/virt-host-validate.rst endif ! WITH_HOST_VALIDATE Non-blocking stuff: The build process spits out noise like this now, but I didn't investigate, maybe it was there before and I missed it: I/O error : Attempt to load network entity http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd manpages/virkeycode-osx.html.in:2: warning: failed to load external entity "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd; 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd; As for the format, there's some improvements and some worse things, most are minor. The one place it's pretty ugly is virt-admin and virsh, with the command format immediately after the command header. Old style looks like: quit, exit quit this interactive terminal Now it is: quit, exit quit exit quit this interactive terminal For larger commands it's better, so it's mostly noticeable at the start of the document with the short commands. Maybe drop the shell section entirely for short style commands, or maybe there's some other option, I didn't look into it much The other bit is nested Example: sections with ~ underline. It puts the Example: at the same indent as the top level command, which is ugly and tough to read. Just grep the rst for '' and see how it manifests in the output manpage. I think those few sections can be replaced by boldified text with *Example:* and it looks better, at least with the man page, but I didn't review the HTML. - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] docs: fix duplication variable name for rst files
On 12/6/19 12:26 PM, Daniel P. Berrangé wrote: > Signed-off-by: Daniel P. Berrangé > --- > docs/Makefile.am | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > Reviewed-by: Cole Robinson - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Revert "qemu: directly create virResctrlInfo ignoring capabilities"
On 12/10/19 5:26 AM, Daniel P. Berrangé wrote: > This reverts commit 7be5fe66cd024b9ffba7c52cdbf5effedeac2c0d. > > This commit broke resctrl, because it missed the fact that the > virResctrlInfoGetCache() has side-effects causing it to actually > change the virResctrlInfo parameter, not merely get data from > it. > > This code will need some refactoring before we can try separating > it from virCapabilities again. Reviewed-by: Cole Robinson - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 00/26] move qemucaps validations from qemu_command to qemu_domain
On 12/10/19 5:00 AM, Peter Krempa wrote: On Mon, Dec 09, 2019 at 20:15:05 -0300, Daniel Henrique Barboza wrote: (series based on master commit 97cafa610ecf5) This work was proposed by Cole in [1]. This is Cole's reasoning for it, copy/pasted from [1]: Nice work. Doing this was long overdue. My only suggestion is that after this we should move all validation into a separate file. qemu_domain was a code dumping place for a long time and since we now have a lot of common code moving it out would be benficial for cleaning up an making it more obvious. Got it. We can create this new file and move the validations from qemu_domain.c to the new file, before resuming moving more code out of qemu_command.c. That way we avoid moving the code twice. Thanks, DHB -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 00/26] move qemucaps validations from qemu_command to qemu_domain
On 12/10/19 6:41 PM, Cole Robinson wrote: On 12/9/19 6:15 PM, Daniel Henrique Barboza wrote: (series based on master commit 97cafa610ecf5) [...] The hotplug code should already be calling into Validate code. When a device is hotplugged via qemu_driver, we get: qemu_driver.c -> qemuDomainAttachDeviceFlags -> qemuDomainAttachDeviceLiveAndConfig -> virDomainDeviceDefParse -> virDomainDeviceDefValidate -> deviceValidateCallback -> qemuDomainDeviceDefValidate So if device validation is moved into the correct qemuDomainDeviceDefValidateXXX function it will get called in the hotplug path so they won't be lost. Good to know. I assumed that the hotplug path was separated. Perhaps this info can be put somewhere in the docs folder as reference. This appears to be the kind of call hierarchy that doesn't change that often, so it wouldn't be a burden to keep the doc updated. - moving CPU Model validation is trickier than the rest because there are code in DefPostParse() that makes CPU Model changes that are then validated in qemu_command.c. Moving the validation to define time doesn't cut in this case - the validation is considering PostParse changes in the CPU Model and some of the will fail if done by qemuDomainDefValidate time. I didn't think too much about how to handle this case, but given that the approach would be different from the other cases handled here, I left it out too. Hmm I glanced at the qemuBuildCpuModelArgStr checks but it doesn't strike me why those are an issue. Validate should always be triggered after PostParse, so the ordering of those two shouldn't impact things here. But I didn't attempt the change Unfortunately I've erased the error I was seeing in this case. I recall something to do with VIR_CPU_MODE_HOST_MODEL being set in qemuDomainDefSetDefaultCPU(), but TBH it's easier to just move the code again and see what happens. I suspect a bug on your side. qemu_command.c has: I'll attempt that in the next iterations of this work. Thanks, Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 00/26] move qemucaps validations from qemu_command to qemu_domain
On 12/9/19 6:15 PM, Daniel Henrique Barboza wrote: > (series based on master commit 97cafa610ecf5) > > This work was proposed by Cole in [1]. This is Cole's reasoning for > it, copy/pasted from [1]: > > --- > The benefits of moving to validate time is that XML is rejected by > 'virsh define' rather than at 'virsh start' time. It also makes it easier > to follow the cli building code, and makes it easier to verify qemu_command.c > test suite code coverage for the important stuff like covering every CLI > option. It's also a good intermediate step for sharing validation with > domain capabilities building, like is done presently for rng models. > --- > > Cole also mentioned that the machine features validation was a good > place to start. I took it a step further and did it across the > board on all qemu_command.c. > > This series didn't create any new validation, just moved them > to domain define time. Any other outcome is unintended. > > > Not all cases where covered in this work. For a first version > I decided to do only the most trivial cases. These are the > other cases I left out for another day: > Yes I think that's the right approach, handle the easy bits first and deal the complicated bits in their own series. Some cases will likely cause a lot of test suite churn, some cases may not even be worth it, we will see! > - all verifications contained in functions that are also called > by qemu_hotplug.c. This means that the hotplug process will also > use the validation code, and we can't just move it to qemu_domain.c. > Besides, not all validation done in domain define time applies > to hotplug, so it's not simply a matter of calling the same function > from qemu_domain in qemu_hotplug. I have patches that moved the > verification of all virtio options to qemu_domain.c, while also > considering qemu_hotplug validation. In the end I decided to leave > it away from this work for now because (1) it took 8 patches just > for virtio case alone and (2) there are a lot of these cases in > qemu_command.c and it would be too much to do it all at in this > same series. > The hotplug code should already be calling into Validate code. When a device is hotplugged via qemu_driver, we get: qemu_driver.c -> qemuDomainAttachDeviceFlags -> qemuDomainAttachDeviceLiveAndConfig -> virDomainDeviceDefParse -> virDomainDeviceDefValidate -> deviceValidateCallback -> qemuDomainDeviceDefValidate So if device validation is moved into the correct qemuDomainDeviceDefValidateXXX function it will get called in the hotplug path so they won't be lost. > - moving CPU Model validation is trickier than the rest because > there are code in DefPostParse() that makes CPU Model changes that > are then validated in qemu_command.c. Moving the validation to define > time doesn't cut in this case - the validation is considering > PostParse changes in the CPU Model and some of the will fail if > done by qemuDomainDefValidate time. I didn't think too much about > how to handle this case, but given that the approach would be > different from the other cases handled here, I left it out too. > Hmm I glanced at the qemuBuildCpuModelArgStr checks but it doesn't strike me why those are an issue. Validate should always be triggered after PostParse, so the ordering of those two shouldn't impact things here. But I didn't attempt the change > - SHMem: part of SHMem validation is being used by hotplug code. > I could've moved the non-hotplug validation to define time, > instead I decided it's better to to leave it to do it all at > once in the future. > shmem is just another device, so it should be fine as long as its final location is triggered by qemuDomainDeviceDefValidate > - DBus-VMState: validation of this fella must be done by first > querying if the hash vm->priv->dbusVMState has elements. > qemuDomainDefValidate does not have 'vm->priv' in it's API, > meaning that I would need to either change the API to include > it (which means changing domainValidateCallback), or do the > validation by another branch where I can get access to vm->priv. > Yeah this one is a little convoluted. The code is using the vm->priv check to determine 'the user requested external slirp', which is really a factor of interface type='user' + a qemu.conf setting, which we _can_ check at validate time against qemuCaps. But it's minor so I suggest just ignoring it > - vmcoreinfo: there are no challenges in moving vmcoreinfo > validation to qemuDomainDefValidate. The problem were the > unit tests. Moving this validation to domain define time > breaks 925 tests on qemuxml2xmltest.c, including tests labelled > as 'minimal' which should pass under any circurstances, as > far as I understand. It is possible to just shoehorn > QEMU_CAPS_DEVICE_VMCOREINFO in the qemuCaps of all tests, but > this would tamper the idea of having to deal with NONE or > LATEST or a specific set of capabilities for certain tests. > Another
Re: [libvirt] [PATCH v3 30/30] virsh: Introduce nvme disk to domblklist
On 12/2/19 9:26 AM, Michal Privoznik wrote: > This is slightly more complicated because NVMe disk source is not > a simple attribute to element. The format in which the > PCI address and namespace ID are printed is the same as QEMU > accepts them: > > nvme://:XX:XX.X/X > > Signed-off-by: Michal Privoznik Reviewed-by: Cole Robinson - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 29/30] qemu_hotplug: Prepare NVMe disks on hotplug
On 12/2/19 9:26 AM, Michal Privoznik wrote: > Signed-off-by: Michal Privoznik > --- > src/qemu/qemu_domain.c | 58 + > src/qemu/qemu_hostdev.c | 22 > src/qemu/qemu_hostdev.h | 6 + > 3 files changed, 86 insertions(+) Reviewed-by: Cole Robinson - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 28/30] qemu: Allow forcing VFIO when computing memlock limit
On 12/2/19 9:26 AM, Michal Privoznik wrote: > With NVMe disks, one can start a blockjob with a NVMe disk > that is not visible in domain XML (at least right away). Usually, > it's fairly easy to override this limitation of > qemuDomainGetMemLockLimitBytes() - for instance for hostdevs we > temporarily add the device to domain def, let the function > calculate the limit and then remove the device. But it's not so > easy with virStorageSourcePtr - in some cases they don't > necessarily are attached to a disk. And even if they are it's > done later in the process and frankly, I find it too complicated > to be able to use the simple trick we use with hostdevs. > > Signed-off-by: Michal Privoznik Reviewed-by: Cole Robinson - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 27/30] qemu: Don't leak storage perms on failure in qemuDomainAttachDiskGeneric
On 12/2/19 9:26 AM, Michal Privoznik wrote: > At the very beginning of the attach function the > qemuDomainStorageSourceChainAccessAllow() is called which > modifies CGroups, locks and seclabels for new disk and its > backing chain. This must be followed by a counterpart which > reverts back all the changes if something goes wrong. This boils > down to calling qemuDomainStorageSourceChainAccessRevoke() which > is done under 'error' label. But not all failure branches jump > there. They just jump onto 'cleanup' label where no revoke is > done. Such mistake is easy to do because 'cleanup' label does > exist. Therefore, dissolve 'error' block in 'cleanup' and have > everything jump onto 'cleanup' label. > > Signed-off-by: Michal Privoznik Reviewed-by: Cole Robinson - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 26/30] qemu_monitor_text: Catch IOMMU/VFIO related errors in qemuMonitorTextAddDrive
On 12/2/19 9:26 AM, Michal Privoznik wrote:
> Because this is a HMP we're dealing with, there is nothing like
> class of reply message, so we have to do some string comparison
> to guess if the command fails. Well, with NVMe disks whole new
> class of errors comes to play because qemu needs to initialize
> IOMMU and VFIO for them. You can see all the messages it may
> produce in qemu_vfio_init_pci().
>
> Signed-off-by: Michal Privoznik
> ---
> src/qemu/qemu_monitor_text.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
> index 9054682d60..6948a5bf90 100644
> --- a/src/qemu/qemu_monitor_text.c
> +++ b/src/qemu/qemu_monitor_text.c
> @@ -75,6 +75,13 @@ int qemuMonitorTextAddDrive(qemuMonitorPtr mon,
> goto cleanup;
> }
>
> +if (strstr(reply, "IOMMU") ||
> +strstr(reply, "VFIO")) {
> +virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> + reply);
> +goto cleanup;
> +}
> +
> ret = 0;
>
> cleanup:
>
For the code:
Reviewed-by: Cole Robinson
Does the blockdev infrastructure have magic that turns the Props into a
drive string? I don't see any -drive examples in the test output
- Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 24/30] qemu_capabilities: Introduce QEMU_CAPS_DRIVE_NVME
On 12/2/19 9:26 AM, Michal Privoznik wrote: > This capability tracks if qemu is capable of: > > -drive file.driver=nvme > > The feature was added in QEMU's commit of v2.12.0-rc0~104^2~2. > > Signed-off-by: Michal Privoznik Reviewed-by: Cole Robinson - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 25/30] qemu: Generate command line of NVMe disks
On 12/2/19 9:26 AM, Michal Privoznik wrote: > Now, that we have everything prepared, we can generate command > line for NVMe disks. > > Signed-off-by: Michal Privoznik > --- > src/qemu/qemu_block.c | 25 +++- > src/qemu/qemu_command.c | 3 + > src/qemu/qemu_process.c | 7 +++ > .../disk-nvme.x86_64-latest.args | 63 +++ > tests/qemuxml2argvtest.c | 1 + > 5 files changed, 98 insertions(+), 1 deletion(-) > create mode 100644 tests/qemuxml2argvdata/disk-nvme.x86_64-latest.args Reviewed-by: Cole Robinson - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 23/30] virSecuritySELinuxRestoreImageLabelInt: Don't skip non-local storage
On 12/2/19 9:26 AM, Michal Privoznik wrote:
> This function is currently not called for any type of storage
> source that is not considered 'local' (as defined by
> virStorageSourceIsLocalStorage()). Well, NVMe disks are not
> 'local' from that point of view and therefore we will need to
> call this function more frequently.
>
> Signed-off-by: Michal Privoznik
> ---
> src/security/security_apparmor.c | 25
> src/security/security_dac.c | 30 +++
> src/security/security_selinux.c | 51 +++-
> 3 files changed, 92 insertions(+), 14 deletions(-)
>
> diff --git a/src/security/security_apparmor.c
> b/src/security/security_apparmor.c
> index 21560b2330..7cea788931 100644
> --- a/src/security/security_apparmor.c
> +++ b/src/security/security_apparmor.c
> @@ -779,9 +779,8 @@ AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr,
>virSecurityDomainImageLabelFlags flags
> G_GNUC_UNUSED)
> {
> virSecurityLabelDefPtr secdef;
> -
> -if (!src->path || !virStorageSourceIsLocalStorage(src))
> -return 0;
> +g_autofree char *vfioGroupDev = NULL;
> +const char *path;
>
> secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_APPARMOR_NAME);
> if (!secdef || !secdef->relabel)
> @@ -790,15 +789,29 @@ AppArmorSetSecurityImageLabel(virSecurityManagerPtr mgr,
> if (!secdef->imagelabel)
> return 0;
>
> +if (src->type == VIR_STORAGE_TYPE_NVME) {
> +const virStorageSourceNVMeDef *nvme = src->nvme;
> +
> +if (!(vfioGroupDev =
> virPCIDeviceAddressGetIOMMUGroupDev(>pciAddr)))
> +return -1;
> +
> +path = vfioGroupDev;
> +} else {
> +if (!src->path || !virStorageSourceIsLocalStorage(src))
> +return 0;
> +
> +path = src->path;
> +}
> +
> /* if the device doesn't exist, error out */
> -if (!virFileExists(src->path)) {
> +if (!virFileExists(path)) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("\'%s\' does not exist"),
> - src->path);
> + path);
> return -1;
> }
>
> -return reload_profile(mgr, def, src->path, true);
> +return reload_profile(mgr, def, path, true);
> }
>
> static int
> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
> index a9a1fad5d7..411aa1b159 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c
> @@ -911,6 +911,19 @@
> virSecurityDACSetImageLabelInternal(virSecurityManagerPtr mgr,
> return -1;
> }
>
> +/* This is not very clean. But so far we don't have NVMe
> + * storage pool backend so that its chownCallback would be
> + * called. And this place looks least offensive. */
> +if (src->type == VIR_STORAGE_TYPE_NVME) {
> +const virStorageSourceNVMeDef *nvme = src->nvme;
> +g_autofree char *vfioGroupDev = NULL;
> +
> +if (!(vfioGroupDev =
> virPCIDeviceAddressGetIOMMUGroupDev(>pciAddr)))
> +return -1;
> +
> +return virSecurityDACSetOwnership(mgr, NULL, vfioGroupDev, user,
> group, false);
> +}
> +
> /* We can't do restore on shared resources safely. Not even
> * with refcounting implemented in XATTRs because if there
> * was a domain running with the feature turned off the
> @@ -1017,6 +1030,23 @@
> virSecurityDACRestoreImageLabelSingle(virSecurityManagerPtr mgr,
> }
> }
>
> +/* This is not very clean. But so far we don't have NVMe
> + * storage pool backend so that its chownCallback would be
> + * called. And this place looks least offensive. */
> +if (src->type == VIR_STORAGE_TYPE_NVME) {
> +const virStorageSourceNVMeDef *nvme = src->nvme;
> +g_autofree char *vfioGroupDev = NULL;
> +
> +if (!(vfioGroupDev =
> virPCIDeviceAddressGetIOMMUGroupDev(>pciAddr)))
> +return -1;
> +
> +/* Ideally, we would check if there is not another PCI
> + * device within domain def that is in the same IOMMU
> + * group. But we're not doing that for hostdevs yet. */
> +
> +return virSecurityDACRestoreFileLabelInternal(mgr, NULL,
> vfioGroupDev, false);
> +}
> +
> return virSecurityDACRestoreFileLabelInternal(mgr, src, NULL, true);
> }
>
I think my suggested helpers will simplify selinux and apparmor a bit.
Not sure about DAC but I don't fully understand the chownCallback +
transaction stuff.
Looks mechanically sound AFAICT
Reviewed-by: Cole Robinson
- Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 22/30] qemu: Allow NVMe disk in CGroups
On 12/2/19 9:26 AM, Michal Privoznik wrote: > If a domain has an NVMe disk configured, then we need to allow it > on devices CGroup so that qemu can access it. There is one caveat > though - if an NVMe disk is read only we need CGroup to allow > write too. This is because when opening the device, qemu does > couple of ioctl()-s which are considered as write. > > Signed-off-by: Michal Privoznik > --- > src/qemu/qemu_cgroup.c | 97 ++ > 1 file changed, 69 insertions(+), 28 deletions(-) A lot going on here, a bit tough to digest, the extra helpers I suggested should help here. But I didn't notice anything bad Reviewed-by: Cole Robinson - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 20/30] qemu: Create NVMe disk in domain namespace
On 12/2/19 9:26 AM, Michal Privoznik wrote:
> If a domain has an NVMe disk configured, then we need to create
> /dev/vfio/* paths in domain's namespace so that qemu can open
> them.
>
> Signed-off-by: Michal Privoznik
> ---
> src/qemu/qemu_domain.c | 67 --
> 1 file changed, 51 insertions(+), 16 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 7b515b9520..70c4ee8e65 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -13430,16 +13430,29 @@ qemuDomainSetupDisk(virQEMUDriverConfigPtr cfg
> G_GNUC_UNUSED,
> {
> virStorageSourcePtr next;
> char *dst = NULL;
> +bool hasNVMe = false;
> int ret = -1;
>
> for (next = disk->src; virStorageSourceIsBacking(next); next =
> next->backingStore) {
> -if (!next->path || !virStorageSourceIsLocalStorage(next)) {
> -/* Not creating device. Just continue. */
> -continue;
> -}
> +if (next->type == VIR_STORAGE_TYPE_NVME) {
> +g_autofree char *nvmePath = NULL;
>
> -if (qemuDomainCreateDevice(next->path, data, false) < 0)
> -goto cleanup;
> +hasNVMe = true;
> +
> +if (!(nvmePath =
> virPCIDeviceAddressGetIOMMUGroupDev(>nvme->pciAddr)))
> +goto cleanup;
> +
> +if (qemuDomainCreateDevice(nvmePath, data, false) < 0)
> +goto cleanup;
> +} else {
> +if (!next->path || !virStorageSourceIsLocalStorage(next)) {
> +/* Not creating device. Just continue. */
> +continue;
> +}
> +
> +if (qemuDomainCreateDevice(next->path, data, false) < 0)
> +goto cleanup;
> +}
> }
>
I don't see anything wrong with this patch. In the interest of getting
this series into git, here is my:
Reviewed-by: Cole Robinson
But, I'm finding a lot of the 'next->type == VIR_STORAGE_TYPE_NVME'
sprinkled around unfortunate, here and in later patches. It seems like
there's two StorageSource cases here involving local filesystem paths:
1) Does src->path contain the storage location? Answered by
virStorageSourceIsLocalStorage
2) Does src have a local path that the VM needs to access? Until now
this was always the same as #1 && src->path, but not any more.
So maybe add helpers like:
bool
virStorageSourceHasLocalResource(virStorageSourcePtr src)
{
return next->path || next->type == VIR_STORAGE_TYPE_NVME;
}
const char *
virStorageSourceGetLocalResource(virStorageSourcePtr src)
{
if (!virStorageSourceHasLocalResource(src))
if (next->type == NVME)
return strdup(next->path);
}
I think virStorageSourceIsLocalStorage should be renamed too because now
it is ambiguous. Maybe something more explicit like SourceUsesPath, or
SourceIsLocalPath. Naming sucks obviously so suggestions welcome
With those funtions, the loop above becomes:
for (...) {
g_autofree char *path = NULL;
if (!HasLocalResource)
continue
if (!(path = GetLocalResource(...))
error
}
The hasNVMe bit can then be replaced with virStorageSourceChainHasNVMe
security driver usage will be simpler too I think. But maybe I'm missing
some complication
- Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 21/30] qemu: Mark NVMe disks as 'need VFIO'
On 12/2/19 9:26 AM, Michal Privoznik wrote: > There are couple of places where a domain with a VFIO device gets > special treatment: in CGroups when enabling/disabling access to > /dev/vfio/vfio, and when creating/removing nodes in domain mount > namespace. Well, a NVMe disk is a VFIO device too. Fortunately, > we have this qemuDomainNeedsVFIO() function which is the only > place that needs adjustment. > > Signed-off-by: Michal Privoznik > --- Reviewed-by: Cole Robinson - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 19/30] qemu: Take NVMe disks into account when calculating memlock limit
On 12/2/19 9:26 AM, Michal Privoznik wrote:
> We have this beautiful function that does crystal ball
> divination. The function is named
> qemuDomainGetMemLockLimitBytes() and it calculates the upper
> limit of how much locked memory is given guest going to need. The
> function bases its guess on devices defined for a domain. For
> instance, if there is a VFIO hostdev defined then it adds 1GiB to
> the guessed maximum. Since NVMe disks are pretty much VFIO
> hostdevs (but not quite), we have to do the same sorcery.
>
> Signed-off-by: Michal Privoznik
> ACKed-by: Peter Krempa
> ---
> src/qemu/qemu_domain.c | 14 --
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
For the patch as-is
Reviewed-by: Cole Robinson
But...
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 59e98de2d2..7b515b9520 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -11920,6 +11920,9 @@ getPPC64MemLockLimitBytes(virDomainDefPtr def)
> }
> }
>
> +if (virDomainDefHasNVMeDisk(def))
> +usesVFIO = true;
> +
We have qemuDomainNeedsVFIO which helpully captures these rules. Looks
the logic has already diverged for ppc here because it doesn't consider
IsMdev devices. Something for a future patch
- Cole
> memory = virDomainDefGetMemoryTotal(def);
>
> if (def->mem.max_memory)
> @@ -12018,6 +12021,7 @@ unsigned long long
> qemuDomainGetMemLockLimitBytes(virDomainDefPtr def)
> {
> unsigned long long memKB = 0;
> +bool usesVFIO = false;
> size_t i;
>
> /* prefer the hard limit */
> @@ -12058,11 +12062,17 @@ qemuDomainGetMemLockLimitBytes(virDomainDefPtr def)
> for (i = 0; i < def->nhostdevs; i++) {
> if (virHostdevIsVFIODevice(def->hostdevs[i]) ||
> virHostdevIsMdevDevice(def->hostdevs[i])) {
> -memKB = virDomainDefGetMemoryTotal(def) + 1024 * 1024;
> -goto done;
> +usesVFIO = true;
> +break;
> }
> }
>
> +if (virDomainDefHasNVMeDisk(def))
> +usesVFIO = true;
> +
> +if (usesVFIO)
> +memKB = virDomainDefGetMemoryTotal(def) + 1024 * 1024;
> +
> done:
> return memKB << 10;
> }
>
- Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 18/30] qemu: prepare NVMe devices too
On 12/2/19 9:26 AM, Michal Privoznik wrote: > The qemu driver has its own wrappers around virHostdev module (so > that some arguments are filled in automatically). Extend these to > include NVMe devices too. > > Signed-off-by: Michal Privoznik > ACKed-by: Peter Krempa > --- > src/qemu/qemu_hostdev.c | 49 ++--- > src/qemu/qemu_hostdev.h | 10 + > 2 files changed, 56 insertions(+), 3 deletions(-) Reviewed-by: Cole Robinson - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/30] conf: move virt type / os type / arch validation to post-parse
On Tue, Dec 10, 2019 at 11:32:18AM -0500, Cole Robinson wrote:
> On 12/10/19 5:57 AM, Daniel P. Berrangé wrote:
> >
> >> Another issue: other drivers use the machine= setting too, libxl at
> >> least. This appears to drop filling in a default machine type for them,
> >> at least at XML parse time.
> >
> > I thought that was the case too, but I couln't find any functional use
> > of def->os.machine in any of the other drivers:
> >
> > $ git grep os.machine | grep -v -E '(qemu|conf)/'
> > libvirt-domain.c: * "os.machine" - the machine hardware name as a
> > string
> > libxl/libxl_conf.c:def->os.machine);
> > libxl/xen_common.c:def->os.machine = g_strdup(capsdata->machinetype);
> > vbox/vbox_common.c:VIR_DEBUG("def->os.machine %s",
> > def->os.machine);
> > vz/vz_sdk.c:if (def->os.machine != NULL || def->os.bootmenu != 0 ||
> >
> >
> > that libxl_conf.c usage is just an error message
> >
> > The xen_common.c usage is just when parsing XM/XL config files,
> > so output only.
> >
> > The vz_sdk.c usage reports error for any non-NULL machine
> >
> >
> > I guess there is a difference in that if the user does dumpxml for a
> > libxl guest we've no longer filled in the machine. I can fix that,
> > but it doesn't look like it'll have a functional effect.
>
> Check grep for 'xenfv' and 'xenpv', domaincapabilities does some machine
> value handling, and virt-manager will use the reported VM machine value
> and feed it back to domaincapabilities, so it could be used in a round
> about way.
Yes, I can see the dom capabilities reports them, and apps can of
coure put them in the XML they feed in, but AFAICT the libxl driver
will not do anything with the values provided when defining/starting
a guest
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] [jenkins-ci PATCH v2 8/8] guests: Build projects on CentOS 8
On Tue, 2019-12-10 at 16:38 +0100, Fabiano Fidêncio wrote: > Signed-off-by: Fabiano Fidêncio > --- > guests/playbooks/build/jobs/defaults.yml| 2 ++ > guests/playbooks/build/projects/libosinfo.yml | 1 + > guests/playbooks/build/projects/libvirt-dbus.yml| 2 ++ > guests/playbooks/build/projects/libvirt.yml | 1 + > guests/playbooks/build/projects/osinfo-db-tools.yml | 1 + > guests/playbooks/build/projects/virt-manager.yml| 3 +++ > jenkins/jobs/defaults.yaml | 2 ++ > jenkins/projects/libosinfo.yaml | 1 + > jenkins/projects/libvirt-dbus.yaml | 2 ++ > jenkins/projects/libvirt.yaml | 1 + > jenkins/projects/osinfo-db-tools.yaml | 1 + > jenkins/projects/virt-manager.yaml | 3 +++ > 12 files changed, 20 insertions(+) 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] [RFC PATCH 1/9] vfio/pci: introduce mediate ops to intercept vfio-pci ops
On Mon, 9 Dec 2019 21:44:23 -0500
Yan Zhao wrote:
> > > > > Currently, yes, i40e has build dependency on vfio-pci.
> > > > > It's like this, if i40e decides to support SRIOV and compiles in vf
> > > > > related code who depends on vfio-pci, it will also have build
> > > > > dependency
> > > > > on vfio-pci. isn't it natural?
> > > >
> > > > No, this is not natural. There are certainly i40e VF use cases that
> > > > have no interest in vfio and having dependencies between the two
> > > > modules is unacceptable. I think you probably want to modularize the
> > > > i40e vfio support code and then perhaps register a table in vfio-pci
> > > > that the vfio-pci code can perform a module request when using a
> > > > compatible device. Just and idea, there might be better options. I
> > > > will not accept a solution that requires unloading the i40e driver in
> > > > order to unload the vfio-pci driver. It's inconvenient with just one
> > > > NIC driver, imagine how poorly that scales.
> > > >
> > > what about this way:
> > > mediate driver registers a module notifier and every time when
> > > vfio_pci is loaded, register to vfio_pci its mediate ops?
> > > (Just like in below sample code)
> > > This way vfio-pci is free to unload and this registering only gives
> > > vfio-pci a name of what module to request.
> > > After that,
> > > in vfio_pci_open(), vfio-pci requests the mediate driver. (or puts
> > > the mediate driver when mediate driver does not support mediating the
> > > device)
> > > in vfio_pci_release(), vfio-pci puts the mediate driver.
> > >
> > > static void register_mediate_ops(void)
> > > {
> > > int (*func)(struct vfio_pci_mediate_ops *ops) = NULL;
> > >
> > > func = symbol_get(vfio_pci_register_mediate_ops);
> > >
> > > if (func) {
> > > func(_dt_ops);
> > > symbol_put(vfio_pci_register_mediate_ops);
> > > }
> > > }
> > >
> > > static int igd_module_notify(struct notifier_block *self,
> > > unsigned long val, void *data)
> > > {
> > > struct module *mod = data;
> > > int ret = 0;
> > >
> > > switch (val) {
> > > case MODULE_STATE_LIVE:
> > > if (!strcmp(mod->name, "vfio_pci"))
> > > register_mediate_ops();
> > > break;
> > > case MODULE_STATE_GOING:
> > > break;
> > > default:
> > > break;
> > > }
> > > return ret;
> > > }
> > >
> > > static struct notifier_block igd_module_nb = {
> > > .notifier_call = igd_module_notify,
> > > .priority = 0,
> > > };
> > >
> > >
> > >
> > > static int __init igd_dt_init(void)
> > > {
> > > ...
> > > register_mediate_ops();
> > > register_module_notifier(_module_nb);
> > > ...
> > > return 0;
> > > }
> >
> >
> > No, this is bad. Please look at MODULE_ALIAS() and request_module() as
> > used in the vfio-platform for loading reset driver modules. I think
> > the correct approach is that vfio-pci should perform a request_module()
> > based on the device being probed. Having the mediation provider
> > listening for vfio-pci and registering itself regardless of whether we
> > intend to use it assumes that we will want to use it and assumes that
> > the mediation provider module is already loaded. We should be able to
> > support demand loading of modules that may serve no other purpose than
> > providing this mediation. Thanks,
> hi Alex
> Thanks for this message.
> So is it good to create a separate module as mediation provider driver,
> and alias its module name to "vfio-pci-mediate-vid-did".
> Then when vfio-pci probes the device, it requests module of that name ?
I think this would give us an option to have the mediator as a separate
module, but not require it. Maybe rather than a request_module(),
where if we follow the platform reset example we'd then expect the init
code for the module to register into a list, we could do a
symbol_request(). AIUI, this would give us a reference to the symbol
if the module providing it is already loaded, and request a module
(perhaps via an alias) if it's not already load. Thanks,
Alex
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [patch v2 1/1] virt-aa-helper: Add support for smartcard host-certificates
On 12/5/19 12:11 PM, Arnaud Patard wrote:
> When emulating smartcard with host certificates, qemu needs to
> be able to read the certificates files. Add necessary code to
> add the smartcard certificates file path to the apparmor profile.
>
> Passthrough support has been tested with spicevmc and remote-viewer.
>
> v2:
> - Fix CodingStyle
> - Add support for 'host' case.
> - Add a comment to mention that the passthrough case doesn't need
> some configuration
> - Use one rule with '{,*}' instead of two rules.
>
> Signed-off-by: Arnaud Patard
> Index: libvirt/src/security/virt-aa-helper.c
> ===
> --- libvirt.orig/src/security/virt-aa-helper.c
> +++ libvirt/src/security/virt-aa-helper.c
> @@ -1271,6 +1271,39 @@ get_files(vahControl * ctl)
> }
> }
>
> +for (i = 0; i < ctl->def->nsmartcards; i++) {
> +virDomainSmartcardDefPtr sc = ctl->def->smartcards[i];
> +virDomainSmartcardType sc_type = sc->type;
> +char *sc_db = (char *)VIR_DOMAIN_SMARTCARD_DEFAULT_DATABASE;
> +if (sc->data.cert.database)
> +sc_db = sc->data.cert.database;
> +switch (sc_type) {
> +/*
> + * Note: At time of writing, to get this working, qemu seccomp
> sandbox has
> + * to be disabled or the host must be running QEMU with commit
> + * 9a1565a03b79d80b236bc7cc2dbce52a2ef3a1b8.
> + * It's possibly due to libcacard:vcard_emul_new_event_thread(),
> which calls
> + * PR_CreateThread(), which calls {g,s}etpriority(). And
> resourcecontrol seccomp
> + * filter forbids it (cf src/qemu/qemu_command.c which seems to
> always use
> + * resourcecontrol=deny).
> + */
This doesn't seem like the type of thing to track in a permanent code
comment, nor a commit message, but as part of the email discussion.
Otherwise, for the code because I don't have a test setup:
Reviewed-by: Cole Robinson
If apparmor maintainers agree they can strip out of the comment so
doesn't require a repost either way IMO
- Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH 4/9] vfio-pci: register default dynamic-trap-bar-info region
On Tue, 10 Dec 2019 02:44:44 -0500 Yan Zhao wrote: > On Tue, Dec 10, 2019 at 05:16:08AM +0800, Alex Williamson wrote: > > On Mon, 9 Dec 2019 01:22:12 -0500 > > Yan Zhao wrote: > > > > > On Fri, Dec 06, 2019 at 11:20:38PM +0800, Alex Williamson wrote: > > > > On Fri, 6 Dec 2019 01:04:07 -0500 > > > > Yan Zhao wrote: > > > > > > > > > On Fri, Dec 06, 2019 at 07:55:30AM +0800, Alex Williamson wrote: > > > > > > On Wed, 4 Dec 2019 22:26:50 -0500 > > > > > > Yan Zhao wrote: > > > > > > > > > > > > > Dynamic trap bar info region is a channel for QEMU and vendor > > > > > > > driver to > > > > > > > communicate dynamic trap info. It is of type > > > > > > > VFIO_REGION_TYPE_DYNAMIC_TRAP_BAR_INFO and subtype > > > > > > > VFIO_REGION_SUBTYPE_DYNAMIC_TRAP_BAR_INFO. > > > > > > > > > > > > > > This region has two fields: dt_fd and trap. > > > > > > > When QEMU detects a device regions of this type, it will create an > > > > > > > eventfd and write its eventfd id to dt_fd field. > > > > > > > When vendor drivre signals this eventfd, QEMU reads trap field of > > > > > > > this > > > > > > > info region. > > > > > > > - If trap is true, QEMU would search the device's PCI BAR > > > > > > > regions and disable all the sparse mmaped subregions (if the > > > > > > > sparse > > > > > > > mmaped subregion is disablable). > > > > > > > - If trap is false, QEMU would re-enable those subregions. > > > > > > > > > > > > > > A typical usage is > > > > > > > 1. vendor driver first cuts its bar 0 into several sections, all > > > > > > > in a > > > > > > > sparse mmap array. So initally, all its bar 0 are passthroughed. > > > > > > > 2. vendor driver specifys part of bar 0 sections to be disablable. > > > > > > > 3. on migration starts, vendor driver signals dt_fd and set trap > > > > > > > to true > > > > > > > to notify QEMU disabling the bar 0 sections of disablable flags > > > > > > > on. > > > > > > > 4. QEMU disables those bar 0 section and hence let vendor driver > > > > > > > be able > > > > > > > to trap access of bar 0 registers and make dirty page tracking > > > > > > > possible. > > > > > > > 5. on migration failure, vendor driver signals dt_fd to QEMU > > > > > > > again. > > > > > > > QEMU reads trap field of this info region which is false and QEMU > > > > > > > re-passthrough the whole bar 0 region. > > > > > > > > > > > > > > Vendor driver specifies whether it supports dynamic-trap-bar-info > > > > > > > region > > > > > > > through cap VFIO_PCI_DEVICE_CAP_DYNAMIC_TRAP_BAR in > > > > > > > vfio_pci_mediate_ops->open(). > > > > > > > > > > > > > > If vfio-pci detects this cap, it will create a default > > > > > > > dynamic_trap_bar_info region on behalf of vendor driver with > > > > > > > region len=0 > > > > > > > and region->ops=null. > > > > > > > Vvendor driver should override this region's len, flags, rw, mmap > > > > > > > in its > > > > > > > vfio_pci_mediate_ops. > > > > > > > > > > > > TBH, I don't like this interface at all. Userspace doesn't pass > > > > > > data > > > > > > to the kernel via INFO ioctls. We have a SET_IRQS ioctl for > > > > > > configuring user signaling with eventfds. I think we only need to > > > > > > define an IRQ type that tells the user to re-evaluate the sparse > > > > > > mmap > > > > > > information for a region. The user would enumerate the device IRQs > > > > > > via > > > > > > GET_IRQ_INFO, find one of this type where the IRQ info would also > > > > > > indicate which region(s) should be re-evaluated on signaling. The > > > > > > user > > > > > > would enable that signaling via SET_IRQS and simply re-evaluate the > > > > > > > > > > > ok. I'll try to switch to this way. Thanks for this suggestion. > > > > > > > > > > > sparse mmap capability for the associated regions when signaled. > > > > > > > > > > > > > > > > Do you like the "disablable" flag of sparse mmap ? > > > > > I think it's a lightweight way for user to switch mmap state of a > > > > > whole region, > > > > > otherwise going through a complete flow of GET_REGION_INFO and > > > > > re-setup > > > > > region might be too heavy. > > > > > > > > No, I don't like the disable-able flag. At what frequency do we expect > > > > regions to change? It seems like we'd only change when switching into > > > > and out of the _SAVING state, which is rare. It seems easy for > > > > userspace, at least QEMU, to drop the entire mmap configuration and > > > ok. I'll try this way. > > > > > > > re-read it. Another concern here is how do we synchronize the event? > > > > Are we assuming that this event would occur when a user switch to > > > > _SAVING mode on the device? That operation is synchronous, the device > > > > must be in saving mode after the write to device state completes, but > > > > it seems like this might be trying to add an asynchronous dependency. > > > > Will the write to device_state only complete once the user
Re: [libvirt] [PATCH 06/30] conf: move virt type / os type / arch validation to post-parse
On 12/10/19 5:57 AM, Daniel P. Berrangé wrote:
>
>> Another issue: other drivers use the machine= setting too, libxl at
>> least. This appears to drop filling in a default machine type for them,
>> at least at XML parse time.
>
> I thought that was the case too, but I couln't find any functional use
> of def->os.machine in any of the other drivers:
>
> $ git grep os.machine | grep -v -E '(qemu|conf)/'
> libvirt-domain.c: * "os.machine" - the machine hardware name as a string
> libxl/libxl_conf.c:def->os.machine);
> libxl/xen_common.c:def->os.machine = g_strdup(capsdata->machinetype);
> vbox/vbox_common.c:VIR_DEBUG("def->os.machine %s",
> def->os.machine);
> vz/vz_sdk.c:if (def->os.machine != NULL || def->os.bootmenu != 0 ||
>
>
> that libxl_conf.c usage is just an error message
>
> The xen_common.c usage is just when parsing XM/XL config files,
> so output only.
>
> The vz_sdk.c usage reports error for any non-NULL machine
>
>
> I guess there is a difference in that if the user does dumpxml for a
> libxl guest we've no longer filled in the machine. I can fix that,
> but it doesn't look like it'll have a functional effect.
Check grep for 'xenfv' and 'xenpv', domaincapabilities does some machine
value handling, and virt-manager will use the reported VM machine value
and feed it back to domaincapabilities, so it could be used in a round
about way.
Thanks,
Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 0/4] qemu: block: implement optional removal of committed snapshot images
This version incorporates Peter's feedback to v3. Pavel Mores (4): qemu: block: propagate the delete flag to where it can actually be used qemu: block: use the delete flag to delete snapshot images if requested qemu: block: store the delete flag in libvirtd's status XML qemu: block: enable the snapshot image deletion feature src/qemu/qemu_blockjob.c | 50 ++- src/qemu/qemu_blockjob.h | 4 +- src/qemu/qemu_domain.c| 4 ++ src/qemu/qemu_driver.c| 12 - .../blockjob-blockdev-in.xml | 1 + 5 files changed, 67 insertions(+), 4 deletions(-) -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 4/4] qemu: block: enable the snapshot image deletion feature
With all plumbing in place, we can now enable the new functionality.
Signed-off-by: Pavel Mores
---
src/qemu/qemu_driver.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9c07b6b393..0cbc746c22 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18544,10 +18544,10 @@ qemuDomainBlockCommit(virDomainPtr dom,
bool persistjob = false;
bool blockdev = false;
-/* XXX Add support for COMMIT_DELETE */
virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW |
VIR_DOMAIN_BLOCK_COMMIT_ACTIVE |
VIR_DOMAIN_BLOCK_COMMIT_RELATIVE |
+ VIR_DOMAIN_BLOCK_COMMIT_DELETE |
VIR_DOMAIN_BLOCK_COMMIT_BANDWIDTH_BYTES, -1);
if (!(vm = qemuDomainObjFromDomain(dom)))
@@ -18568,6 +18568,13 @@ qemuDomainBlockCommit(virDomainPtr dom,
blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV);
+if (!blockdev && (flags & VIR_DOMAIN_BLOCK_COMMIT_DELETE)) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+ _("deleting committed images is only supported for VMs "
+ "with blockdev enabled"));
+goto endjob;
+}
+
/* Convert bandwidth MiB to bytes, if necessary */
if (!(flags & VIR_DOMAIN_BLOCK_COMMIT_BANDWIDTH_BYTES)) {
if (speed > LLONG_MAX >> 20) {
@@ -18686,7 +18693,8 @@ qemuDomainBlockCommit(virDomainPtr dom,
goto endjob;
if (!(job = qemuBlockJobDiskNewCommit(vm, disk, top_parent, topSource,
- baseSource, false)))
+ baseSource,
+ flags &
VIR_DOMAIN_BLOCK_COMMIT_DELETE)))
goto endjob;
disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 2/4] qemu: block: use the delete flag to delete snapshot images if requested
When blockcommit finishes successfully, one of the
qemuBlockJobProcessEventCompletedCommit() and
qemuBlockJobProcessEventCompletedActiveCommit() event handlers is called.
This is where the delete flag (stored in qemuBlockJobCommitData since the
previous commit) can actually be used to delete the committed snapshot
images if requested.
We use virFileRemove() instead of a simple unlink() to cover the case where
the image to be removed is on an NFS volume.
Signed-off-by: Pavel Mores
---
src/qemu/qemu_blockjob.c | 46
1 file changed, 46 insertions(+)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 718e311213..498e2a716f 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -1004,6 +1004,44 @@ qemuBlockJobProcessEventCompletedPull(virQEMUDriverPtr
driver,
}
+/**
+ * qemuBlockJobDeleteImages:
+ * @driver: qemu driver object
+ * @vm: domain object
+ * @disk: disk object that the chain to be deleted is associated with
+ * @top: top snapshot of the chain to be deleted
+ *
+ * Helper for removing snapshot images. Intended for callers like
+ * qemuBlockJobProcessEventCompletedCommit() and
+ * qemuBlockJobProcessEventCompletedActiveCommit() as it relies on adjustments
+ * these functions perform on the 'backingStore' chain to function correctly.
+ *
+ * TODO look into removing backing store for non-local snapshots too
+ */
+static void
+qemuBlockJobDeleteImages(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainDiskDefPtr disk,
+ virStorageSourcePtr top)
+{
+virStorageSourcePtr p = top;
+g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+uid_t uid;
+gid_t gid;
+
+for (; p != NULL; p = p->backingStore) {
+if (virStorageSourceGetActualType(p) == VIR_STORAGE_TYPE_FILE) {
+
+qemuDomainGetImageIds(cfg, vm, p, disk->src, , );
+
+if (virFileRemove(p->path, uid, gid) < 0) {
+VIR_WARN("Unable to remove snapshot image file '%s' (%s)",
+ p->path, g_strerror(errno));
+}
+}
+}
+}
+
/**
* qemuBlockJobProcessEventCompletedCommit:
* @driver: qemu driver object
@@ -1070,6 +1108,10 @@ qemuBlockJobProcessEventCompletedCommit(virQEMUDriverPtr
driver,
job->data.commit.topparent->backingStore = job->data.commit.base;
qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob,
job->data.commit.top);
+
+if (job->data.commit.deleteCommittedImages)
+qemuBlockJobDeleteImages(driver, vm, job->disk, job->data.commit.top);
+
virObjectUnref(job->data.commit.top);
job->data.commit.top = NULL;
@@ -1160,6 +1202,10 @@
qemuBlockJobProcessEventCompletedActiveCommit(virQEMUDriverPtr driver,
job->disk->src->readonly = job->data.commit.top->readonly;
qemuBlockJobEventProcessConcludedRemoveChain(driver, vm, asyncJob,
job->data.commit.top);
+
+if (job->data.commit.deleteCommittedImages)
+qemuBlockJobDeleteImages(driver, vm, job->disk, job->data.commit.top);
+
virObjectUnref(job->data.commit.top);
job->data.commit.top = NULL;
/* the mirror element does not serve functional purpose for the commit job
*/
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 3/4] qemu: block: store the delete flag in libvirtd's status XML
Since blockcommit is asynchronous, libvirtd can be restarted while the
operation runs. To ensure the information necessary to finish up the job
is not lost, serialisation to and deserialisation from the status XML is
added.
To unittest this, the new element was only added to the active commit test,
the non-active commit test doesn't have the new element so as to test its
absence.
Signed-off-by: Pavel Mores
---
src/qemu/qemu_domain.c | 4
tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml | 1 +
2 files changed, 5 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 767790bfc0..27926c7670 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2586,6 +2586,8 @@ qemuDomainObjPrivateXMLFormatBlockjobIterator(void
*payload,
virBufferAsprintf(, "\n",
job->data.commit.top->nodeformat);
if (job->data.commit.topparent)
virBufferAsprintf(, "\n",
job->data.commit.topparent->nodeformat);
+if (job->data.commit.deleteCommittedImages)
+virBufferAddLit(, "\n");
break;
case QEMU_BLOCKJOB_TYPE_CREATE:
@@ -3185,6 +3187,8 @@
qemuDomainObjPrivateXMLParseBlockjobDataSpecific(qemuBlockJobDataPtr job,
"string(./base/@node)",
>data.commit.base,
ctxt);
+if (virXPathNode("./deleteCommittedImages", ctxt))
+job->data.commit.deleteCommittedImages = true;
if (!job->data.commit.top ||
!job->data.commit.base)
goto broken;
diff --git a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml
b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml
index 67ab099bd9..b5d62fd4ab 100644
--- a/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml
+++ b/tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml
@@ -242,6 +242,7 @@
+
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 1/4] qemu: block: propagate the delete flag to where it can actually be used
Propagate the delete flag from qemuDomainBlockCommit() (which was just
ignoring it until now) to qemuBlockJobDiskNewCommit() where it can be
stored in the qemuBlockJobCommitData structure which holds information
necessary to finish the job asynchronously.
In the actual qemuBlockJobDiskNewCommit() in this commit, we temporarily
pass a literal 'false' to preserve the current behaviour until the whole
implementation of the feature is in place.
Signed-off-by: Pavel Mores
---
src/qemu/qemu_blockjob.c | 4 +++-
src/qemu/qemu_blockjob.h | 4 +++-
src/qemu/qemu_driver.c | 2 +-
3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 81aa46c2fb..718e311213 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -282,7 +282,8 @@ qemuBlockJobDiskNewCommit(virDomainObjPtr vm,
virDomainDiskDefPtr disk,
virStorageSourcePtr topparent,
virStorageSourcePtr top,
- virStorageSourcePtr base)
+ virStorageSourcePtr base,
+ bool delete_imgs)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
g_autoptr(qemuBlockJobData) job = NULL;
@@ -305,6 +306,7 @@ qemuBlockJobDiskNewCommit(virDomainObjPtr vm,
job->data.commit.topparent = topparent;
job->data.commit.top = top;
job->data.commit.base = base;
+job->data.commit.deleteCommittedImages = delete_imgs;
if (qemuBlockJobRegister(job, vm, disk, true) < 0)
return NULL;
diff --git a/src/qemu/qemu_blockjob.h b/src/qemu/qemu_blockjob.h
index 52b03aaf9e..42b973fe96 100644
--- a/src/qemu/qemu_blockjob.h
+++ b/src/qemu/qemu_blockjob.h
@@ -87,6 +87,7 @@ struct _qemuBlockJobCommitData {
virStorageSourcePtr topparent;
virStorageSourcePtr top;
virStorageSourcePtr base;
+bool deleteCommittedImages;
};
@@ -180,7 +181,8 @@ qemuBlockJobDiskNewCommit(virDomainObjPtr vm,
virDomainDiskDefPtr disk,
virStorageSourcePtr topparent,
virStorageSourcePtr top,
- virStorageSourcePtr base);
+ virStorageSourcePtr base,
+ bool delete_imgs);
qemuBlockJobDataPtr
qemuBlockJobNewCreate(virDomainObjPtr vm,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9197dffadd..9c07b6b393 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18686,7 +18686,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
goto endjob;
if (!(job = qemuBlockJobDiskNewCommit(vm, disk, top_parent, topSource,
- baseSource)))
+ baseSource, false)))
goto endjob;
disk->mirrorState = VIR_DOMAIN_DISK_MIRROR_STATE_NONE;
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: honour parseOpaque instead of refetching caps
On 12/10/19 7:43 AM, Daniel P. Berrangé wrote:
> The use of the parseOpaque parameter was mistakenly removed in
>
> commit 4a4132b4625778cf80acb9c92d06351b44468ac3
> Author: Daniel P. Berrangé
> Date: Tue Dec 3 10:49:49 2019 +
>
> conf: don't use passed in caps in post parse method
>
> causing the method to re-fetch qemuCaps that were already just
> fetched and put into parseOpaque.
>
> This is inefficient when parsing incoming XML, but for live
> XML this is more serious as it means we use the capabilities
> for the current QEMU binary on disk, rather than the running
> QEMU.
>
> That commit, however, did have a useful side effect of fixing
> a crasher bug in the qemu post parse callback introduced by
>
> commit 5e939cea896fb3373a6f68f86e325c657429ed3d
> Author: Jiri Denemark
> Date: Thu Sep 26 18:42:02 2019 +0200
>
> qemu: Store default CPU in domain XML
>
> The qemuDomainDefSetDefaultCPU() method in that patch did not
> allow for the possibility that qemuCaps would be NULL and thus
> resulted in a SEGV.
>
> This shows a risk in letting each check in the post parse
> callback look for qemuCaps == NULL. The safer option is to
> check once upfront and immediately stop (postpone) further
> validation.
>
> Honouring the cached caps for the live status XML, highlights
> a second flaw, which is that it shouldn't check the virt
> type or arch for running guests. The info needed to check this
> is not in the cache caps, only the list of flags are populated.
>
> Thus some of the post parse logic is made to only run for
> inactive guests. This showed a bug in one test data file which
> lacked an ID attribute for the live guest.
>
> Signed-off-by: Daniel P. Berrangé
> ---
> src/qemu/qemu_domain.c| 52 +++
> .../qemustatusxml2xmldata/vcpus-multi-in.xml | 2 +-
> 2 files changed, 31 insertions(+), 23 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 6f53e17b6a..2abe93c829 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -4703,16 +4703,17 @@ static int
> qemuDomainDefPostParse(virDomainDefPtr def,
> unsigned int parseFlags,
> void *opaque,
> - void *parseOpaque G_GNUC_UNUSED)
> + void *parseOpaque)
> {
> virQEMUDriverPtr driver = opaque;
> g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
> -g_autoptr(virQEMUCaps) qemuCaps = NULL;
> +/* Note that qemuCaps may be NULL when this function is called. This
> + * function shall not fail in that case. It will be re-run on VM startup
> + * with the capabilities populated. */
> +virQEMUCapsPtr qemuCaps = parseOpaque;
>
> -if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache,
> -def->emulator))) {
> +if (!qemuCaps)
> return 1;
> -}
>
ACK to this bit, but move the comment to be above the if (!qemuCaps) IMO
> if (def->os.type != VIR_DOMAIN_OSTYPE_HVM) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> @@ -4721,18 +4722,31 @@ qemuDomainDefPostParse(virDomainDefPtr def,
> return -1;
> }
>
> -if (!virQEMUCapsIsArchSupported(qemuCaps, def->os.arch)) {
> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> - _("Emulator '%s' does not support arch '%s'"),
> - def->emulator, virArchToString(def->os.arch));
> -return -1;
> -}
> +if (def->id == -1) {
> +if (!virQEMUCapsIsArchSupported(qemuCaps, def->os.arch)) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Emulator '%s' does not support arch '%s'"),
> + def->emulator, virArchToString(def->os.arch));
> +return -1;
> +}
>
> -if (!virQEMUCapsIsVirtTypeSupported(qemuCaps, def->virtType)) {
> -virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> - _("Emulator '%s' does not support virt type '%s'"),
> - def->emulator,
> virDomainVirtTypeToString(def->virtType));
> -return -1;
> +if (!virQEMUCapsIsVirtTypeSupported(qemuCaps, def->virtType)) {
> +virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("Emulator '%s' does not support virt type
> '%s'"),
> + def->emulator,
> virDomainVirtTypeToString(def->virtType));
> +return -1;
> +}
> +if (!def->os.machine) {
> +const char *machine = virQEMUCapsGetPreferredMachine(qemuCaps,
> +
> def->virtType);
> +def->os.machine = g_strdup(machine);
> +}
> +} else {
> +if (!def->os.machine) {
> +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("missing machine
Re: [libvirt] [jenkins-ci PATCH v2 7/8] guests: Add projects to CentOS 8
On Tue, 2019-12-10 at 16:38 +0100, Fabiano Fidêncio wrote: > Signed-off-by: Fabiano Fidêncio > --- > guests/host_vars/libvirt-centos-8/main.yml | 15 +++ > 1 file changed, 15 insertions(+) The cover letter contained the following remark: > The projects which were not added due to missing dependencies are: > - libvirt-cim > - libvirt-sandbox > - libvirt-tck I think it would make perfect sense to include that remark in the commit message, so that it remains as part of the git history. With that added, 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] doc: vtpm only support secrets by UUID at this point
> On 10 Dec 2019, at 16:08, marcandre.lur...@redhat.com wrote: > > From: Marc-André Lureau > > Support by usage name can be considered separately (with a 'usage' > attribute?). > > Cc: Stefan Berger > Signed-off-by: Marc-André Lureau > --- > docs/formatsecret.html.in | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/docs/formatsecret.html.in b/docs/formatsecret.html.in > index 8d0630a7c3..8f5383cf64 100644 > --- a/docs/formatsecret.html.in > +++ b/docs/formatsecret.html.in > @@ -334,8 +334,8 @@ Secret value set > of the vTPM. > The usage type='vtpm' element must contain > a single name element that specifies a usage name > - for the secret. The vTPM secret can then be used by UUID or by > - this usage name via the encryption element of > + for the secret. The vTPM secret can then be used by UUID > + via the encryption element of > a tpm when using an > emulator. > Since 5.6.0. The following is an example > -- > 2.24.0.308.g228f53135a Reminds me of our discussion :-) Reviewed-by: Christophe de Dinechin > > -- > 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] [jenkins-ci PATCH v2 4/8] mappings: Adjustments for CentOS 8
On Tue, 2019-12-10 at 16:38 +0100, Fabiano Fidêncio wrote: > In order to build our projects on a CentOS 8 machine / container, some > changes will be required: > > - libssh2, libvirt dependency, is not longer present: > - > https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/8/html-single/considerations_in_adopting_rhel_8/index#literal_libssh2_literal_is_not_available_in_rhel_8 > > - librbd1-devel, libvirt dependency, has been replaced by librdb-devel: > - > https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/8/html-single/considerations_in_adopting_rhel_8/index#package-replacements_changes-to-packages > > - libgovirt-devel and gtk-vnc2-devel, virt-viewer dependencies, are not > present anymore: > - > https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/8/html-single/considerations_in_adopting_rhel_8/index#removed-packages_changes-to-packages > > Signed-off-by: Fabiano Fidêncio > --- > guests/vars/mappings.yml | 4 > 1 file changed, 4 insertions(+) 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] [jenkins-ci PATCH v2 3/8] guests: Add CentOS 8
On Tue, 2019-12-10 at 16:38 +0100, Fabiano Fidêncio wrote: > This commit allows calling `lcitool install libvirt-centos-8`. > > `lcitool update libvirt-centos-8 $project` does *not* work as: > - EPEL repository is only enabled for CentOS 7; > - The reason EPEL is needed because of some base packages (as screen); > - There's no task to enable PowerTools repository; > - PowerTools is needed because the development packages are > distributed via this repo; > - There's *no* project supported; > > Signed-off-by: Fabiano Fidêncio > --- > guests/host_vars/libvirt-centos-8/docker.yml | 2 ++ > guests/host_vars/libvirt-centos-8/install.yml | 2 ++ > guests/host_vars/libvirt-centos-8/main.yml| 7 +++ > guests/inventory | 1 + > 4 files changed, 12 insertions(+) > create mode 100644 guests/host_vars/libvirt-centos-8/docker.yml > create mode 100644 guests/host_vars/libvirt-centos-8/install.yml > create mode 100644 guests/host_vars/libvirt-centos-8/main.yml 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 v3 10/30] schemas: Introduce disk type NVMe
On 12/9/19 11:55 PM, Cole Robinson wrote: On 12/2/19 9:26 AM, Michal Privoznik wrote: There is this class of PCI devices that act like disks: NVMe. Therefore, they are both PCI devices and disks. While we already have (and can assign a NVMe device to a domain successfully) we don't have disk representation. There are three problems with PCI assignment in case of a NVMe device: 1) domains with can't be migrated 2) NVMe device is assigned whole, there's no way to assign only a namespace 3) Because hypervisors see they don't put block layer on top of it - users don't get all the fancy features like snapshots NVMe namespaces are way of splitting one continuous NVDIMM memory into smaller ones, effectively creating smaller NVMe-s (which can then be partitioned, LVMed, etc.) Because of all of this the following XML was chosen to model a NVMe device: Signed-off-by: Michal Privoznik --- docs/formatdomain.html.in| 57 +++-- docs/schemas/domaincommon.rng| 32 ++ tests/qemuxml2argvdata/disk-nvme.xml | 63 3 files changed, 149 insertions(+), 3 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-nvme.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6df4a8b26e..fe871d933f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2944,6 +2944,13 @@ /backingStore target dev='vdd' bus='virtio'/ /disk + disk type='nvme' device='disk' +driver name='qemu' type='raw'/ +source type='pci' managed='yes' namespace='1' + address domain='0x' bus='0x01' slot='0x00' function='0x0'/ +/source +target dev='vde' bus='virtio'/ + /disk /devices ... @@ -2957,7 +2964,8 @@ Valid values are "file", "block", "dir" (since 0.7.5), "network" (since 0.8.7), or -"volume" (since 1.0.5) +"volume" (since 1.0.5), or +"nvme" (since 5.6.0) 6.0.0 or whatever version this will land in and refer to the underlying source for the disk. Since 0.0.3 @@ -3140,6 +3148,43 @@ Since 1.0.5 +nvme + + To specify disk source for NVMe disk the source + element has the following attributes: + +type +The type of address specified in address +sub-element. Currently, only pci value is +accepted. + + +managed +This attribute instructs libvirt to detach NVMe +controller automatically on domain startup (yes) +or expect the controller to be detached by system +administrator (no). + + +namespace +The namespace ID which should be assigned to the domain. +According to NVMe standard, namespace numbers start from 1, +including. + + + + The difference between disk type='nvme' + and hostdev/ is that the latter is plain + host device assignment with all its limitations (e.g. no live + migration), while the former makes hypervisor to run the NVMe + disk through hypervisor's block layer thus enabling all + features provided by the layer (e.g. snapshots, domain + migration, etc.). Moreover, since the NVMe disk is unbinded + from its PCI driver, the host kernel storage stack is not + involved (compared to passing say /dev/nvme0n1 via + disk type='block' and therefore lower + latencies can be achieved. + With "file", "block", and "volume", one or more optional sub-elements seclabel, described @@ -3302,11 +3347,17 @@ initiator IQN needed to access the source via mandatory attribute name. + address + For disk of type nvme this element +specifies the PCI address of the host NVMe +controller. +Since 5.6.0 Same + -For a "file" or "volume" disk type which represents a cdrom or floppy -(the device attribute), it is possible to define +For a "file" or "volume" disk type which represents a cdrom or +floppy (the device attribute), it is possible to define Stray change? Oh right. I've realigned this area when adding the address description. But this change does not belong here. Also, tn the test XML you need to "s/qemu-system-i686/qemu-system-i386/" or you'll hit a weird error. And VIR_TEST_REGENERATE_OUTPUT is also busted, see my patches elsewhere on this list. Yeah, I've noticed Dan
Re: [libvirt] [PATCH v3 11/30] conf: Format and parse NVMe type disk
On 12/10/19 12:43 AM, Cole Robinson wrote: On 12/2/19 9:26 AM, Michal Privoznik wrote: To simplify implementation, some restrictions are added. For instance, an NVMe disk can't go to any bus but virtio and has to be type of 'disk' and can't have startupPolicy set. Signed-off-by: Michal Privoznik Reviewed-by: Cole Robinson Arguably the nvme->namespace == 0 should be in a Validate callback, but it's extremely minor Fixed. Initially, I thought that namespace == 0 is invalid XML but I can argue both ways. And since you have inclination to the Validate callback I've moved the check there. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 00/26] move qemucaps validations from qemu_command to qemu_domain
On 12/10/19 5:56 AM, Peter Krempa wrote: > On Tue, Dec 10, 2019 at 09:58:38 +, Daniel Berrange wrote: >> On Mon, Dec 09, 2019 at 08:15:05PM -0300, Daniel Henrique Barboza wrote: >>> (series based on master commit 97cafa610ecf5) >>> >>> This work was proposed by Cole in [1]. This is Cole's reasoning for >>> it, copy/pasted from [1]: >>> >>> --- >>> The benefits of moving to validate time is that XML is rejected by >>> 'virsh define' rather than at 'virsh start' time. It also makes it easier >>> to follow the cli building code, and makes it easier to verify >>> qemu_command.c >>> test suite code coverage for the important stuff like covering every CLI >>> option. It's also a good intermediate step for sharing validation with >>> domain capabilities building, like is done presently for rng models. >>> --- >> >> I've not looked at the patches, but surely moving this validate from >> start time, to be define time errors is going to cause functional >> regressions in our ABI behaviour. >> >> My libvirtd daemons installs have many XML files defined which will >> fail validation at various points in time, depending on what QEMU >> builds I happen to have deployed. I only need them to pass the >> validation when actually starting the VM. > > Validation is not done on the persistent or status XML files exactly for > this reason. > > It's re-done when starting the VM so this should not be an issue. > Yup, that's what the VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE flag is all about. It's used to skip validation at daemon startup, and various cases like DomainDefCopy Conceptually this moves closer to IMO the ideal model: 1) Serialize XML into C struct 2) Validate contents against hypervisor capabilities 3) Serialize C struct into command line string Which has benefits: simplifies code in #1 and #3 which is the interesting stuff, easier to test full code coverage of #1 and #3, gives us the option to move validation into domaincapabilities code so we can report to the user whether a feature will be rejected or not. Also I suspect if we moved to using virtblocks for cli stuff, untangling validation from the cli builder would be a step in that direction - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH v2 1/8] lcitool: Generate the unattended script file
Let's always generate the unattended script file as, by doing this, we
can treat the files we have as templates, being able to change them
accordingly to whatever is needed by specific distros.
It's important to note that we *must* be careful and keep generating
those files using their "expected" filename, preferably using the same
name of the template, as some of the distros require that.
Signed-off-by: Fabiano Fidêncio
Reviewed-by: Andrea Bolognani
---
guests/lcitool | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/guests/lcitool b/guests/lcitool
index 8436ce7..379fecc 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -23,9 +23,11 @@ import json
import os
import platform
import random
+import shutil
import string
import subprocess
import sys
+import tempfile
import textwrap
import yaml
@@ -537,7 +539,18 @@ class Application:
raise Exception(
"Host {} doesn't support installation".format(host)
)
-initrd_inject = os.path.join(base, "configs", install_config)
+
+# Unattended install scripts are being generated on the fly, based
+# on the templates present in guests/configs/
+initrd_template = os.path.join(base, "configs", install_config)
+with open(initrd_template, 'r') as template:
+content = template.read()
+
+tempdir = tempfile.mkdtemp()
+initrd_inject = os.path.join(tempdir, install_config)
+
+with open(initrd_inject, "w") as inject:
+inject.write(content)
# preseed files must use a well-known name to be picked up by
# d-i; for kickstart files, we can use whatever name we please
@@ -587,6 +600,8 @@ class Application:
except Exception as ex:
raise Exception("Failed to install '{}': {}".format(host, ex))
+shutil.rmtree(tempdir, ignore_errors=True)
+
def _action_update(self, args):
self._execute_playbook("update", args.hosts, args.projects,
args.git_revision)
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH v2 8/8] guests: Build projects on CentOS 8
Signed-off-by: Fabiano Fidêncio
---
guests/playbooks/build/jobs/defaults.yml| 2 ++
guests/playbooks/build/projects/libosinfo.yml | 1 +
guests/playbooks/build/projects/libvirt-dbus.yml| 2 ++
guests/playbooks/build/projects/libvirt.yml | 1 +
guests/playbooks/build/projects/osinfo-db-tools.yml | 1 +
guests/playbooks/build/projects/virt-manager.yml| 3 +++
jenkins/jobs/defaults.yaml | 2 ++
jenkins/projects/libosinfo.yaml | 1 +
jenkins/projects/libvirt-dbus.yaml | 2 ++
jenkins/projects/libvirt.yaml | 1 +
jenkins/projects/osinfo-db-tools.yaml | 1 +
jenkins/projects/virt-manager.yaml | 3 +++
12 files changed, 20 insertions(+)
diff --git a/guests/playbooks/build/jobs/defaults.yml
b/guests/playbooks/build/jobs/defaults.yml
index 0175a05..5e4ec03 100644
--- a/guests/playbooks/build/jobs/defaults.yml
+++ b/guests/playbooks/build/jobs/defaults.yml
@@ -1,6 +1,7 @@
---
all_machines:
- libvirt-centos-7
+ - libvirt-centos-8
- libvirt-debian-9
- libvirt-debian-10
- libvirt-debian-sid
@@ -15,6 +16,7 @@ all_machines:
- libvirt-ubuntu-1804
rpm_machines:
- libvirt-centos-7
+ - libvirt-centos-8
- libvirt-fedora-30
- libvirt-fedora-31
- libvirt-fedora-rawhide
diff --git a/guests/playbooks/build/projects/libosinfo.yml
b/guests/playbooks/build/projects/libosinfo.yml
index a759204..6391323 100644
--- a/guests/playbooks/build/projects/libosinfo.yml
+++ b/guests/playbooks/build/projects/libosinfo.yml
@@ -13,6 +13,7 @@
# RPM build is still not possible on CentOS7 as it does not
# have the needed RPM macros for meson.
machines:
+ - libvirt-centos-8
- libvirt-fedora-30
- libvirt-fedora-31
- libvirt-fedora-rawhide
diff --git a/guests/playbooks/build/projects/libvirt-dbus.yml
b/guests/playbooks/build/projects/libvirt-dbus.yml
index 8c946af..66bc1fa 100644
--- a/guests/playbooks/build/projects/libvirt-dbus.yml
+++ b/guests/playbooks/build/projects/libvirt-dbus.yml
@@ -14,6 +14,7 @@
# Python3 version in Ubuntu 16.04 and python3-pytest version
# in CentOS 7 are too old.
machines:
+ - libvirt-centos-8
- libvirt-debian-9
- libvirt-debian-10
- libvirt-debian-sid
@@ -25,6 +26,7 @@
# RPM build is still not possible on CentOS7 as it does not
# have the needed RPM macros for meson.
machines:
+ - libvirt-centos-8
- libvirt-fedora-30
- libvirt-fedora-31
- libvirt-fedora-rawhide
diff --git a/guests/playbooks/build/projects/libvirt.yml
b/guests/playbooks/build/projects/libvirt.yml
index e0f2f7c..654d16c 100644
--- a/guests/playbooks/build/projects/libvirt.yml
+++ b/guests/playbooks/build/projects/libvirt.yml
@@ -13,6 +13,7 @@
# commands with more arguments than FreeBSD supports
machines:
- libvirt-centos-7
+ - libvirt-centos-8
- libvirt-debian-9
- libvirt-debian-10
- libvirt-debian-sid
diff --git a/guests/playbooks/build/projects/osinfo-db-tools.yml
b/guests/playbooks/build/projects/osinfo-db-tools.yml
index 18f0f0f..b5024ae 100644
--- a/guests/playbooks/build/projects/osinfo-db-tools.yml
+++ b/guests/playbooks/build/projects/osinfo-db-tools.yml
@@ -13,6 +13,7 @@
# RPM build is still not possible on CentOS7 as it does not
# have the needed RPM macros for meson.
machines:
+ - libvirt-centos-8
- libvirt-fedora-30
- libvirt-fedora-31
- libvirt-fedora-rawhide
diff --git a/guests/playbooks/build/projects/virt-manager.yml
b/guests/playbooks/build/projects/virt-manager.yml
index c0d4294..886cb73 100644
--- a/guests/playbooks/build/projects/virt-manager.yml
+++ b/guests/playbooks/build/projects/virt-manager.yml
@@ -5,6 +5,7 @@
# Ubuntu 16.04 has Python 3 but not the libxml2 bindings, so it can't
# build the project either
machines:
+ - libvirt-centos-8
- libvirt-debian-9
- libvirt-debian-10
- libvirt-debian-sid
@@ -29,6 +30,7 @@
# so skip the test suite there for the time being. See
# https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=224902
machines:
+ - libvirt-centos-8
- libvirt-debian-9
- libvirt-debian-10
- libvirt-debian-sid
@@ -39,6 +41,7 @@
- include: '{{ playbook_base }}/jobs/python-distutils-rpm-job.yml'
vars:
machines:
+ - libvirt-centos-8
- libvirt-fedora-30
- libvirt-fedora-31
- libvirt-fedora-rawhide
diff --git a/jenkins/jobs/defaults.yaml b/jenkins/jobs/defaults.yaml
index 676ecbf..9232d42 100644
--- a/jenkins/jobs/defaults.yaml
+++ b/jenkins/jobs/defaults.yaml
@@ -4,6 +4,7 @@
node: libvirt
all_machines:
- libvirt-centos-7
+ - libvirt-centos-8
- libvirt-debian-9
- libvirt-debian-10
- libvirt-fedora-30
@@ -13,6 +14,7 @@
- libvirt-freebsd-12
rpm_machines:
-
[libvirt] [jenkins-ci PATCH v2 4/8] mappings: Adjustments for CentOS 8
In order to build our projects on a CentOS 8 machine / container, some changes will be required: - libssh2, libvirt dependency, is not longer present: - https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/8/html-single/considerations_in_adopting_rhel_8/index#literal_libssh2_literal_is_not_available_in_rhel_8 - librbd1-devel, libvirt dependency, has been replaced by librdb-devel: - https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/8/html-single/considerations_in_adopting_rhel_8/index#package-replacements_changes-to-packages - libgovirt-devel and gtk-vnc2-devel, virt-viewer dependencies, are not present anymore: - https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/8/html-single/considerations_in_adopting_rhel_8/index#removed-packages_changes-to-packages Signed-off-by: Fabiano Fidêncio --- guests/vars/mappings.yml | 4 1 file changed, 4 insertions(+) diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml index 56310f0..07ab6db 100644 --- a/guests/vars/mappings.yml +++ b/guests/vars/mappings.yml @@ -238,6 +238,7 @@ mappings: deb: libgtk-vnc-2.0-dev pkg: gtk-vnc rpm: gtk-vnc2-devel +CentOS8: cross-policy-deb: foreign hal: @@ -332,6 +333,7 @@ mappings: libgovirt: rpm: libgovirt-devel +CentOS8: Debian: libgovirt-dev cross-policy-deb: foreign @@ -379,6 +381,7 @@ mappings: deb: librbd-dev Fedora: librbd-devel CentOS7: librbd1-devel +CentOS8: librbd-devel OpenSUSE: librbd-devel cross-policy-deb: foreign @@ -404,6 +407,7 @@ mappings: deb: libssh2-1-dev pkg: libssh2 rpm: libssh2-devel +CentOS8: cross-policy-deb: foreign libtirpc: -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH v2 0/8] Add CentOS 8 to libvirt-jenkins-ci
In order to add CentOS 8 to jenkins-ci, we had to: - Adjust lcitool to generate unattended scripts based on templates instead of using the templates themselves, as a new parameter is expected on kickstart files for RHEL / CentOS 8 (or newer); - Enable the EPEL and PowerTools repos; Then, the following projects have been added: - libvirt - Some adjustments had to be done as some dependencies got removed from CentOS 8; - libvirt-glib - libvirt-dbus - Some adjustments had to be done as flake8 doesn't pull all the needed dependencies when installed on CentOS 8; - libvirt-go - libvirt-go-xml - libvirt-ocaml - libvirt-perl - libvirt-python - virt-viewer - Some adjustments had to be done as libgovirt-devel and gtk-vnc2-devel are not present anymore on CentOS 8; - osinfo-db-tools - oisnfo-db - libosinfo - virt-manager The projects which were not added due to missing dependencies are: - libvirt-cim - libvirt-sandbox - libvirt-tck Fabiano Fidêncio (8): lcitool: Generate the unattended script file lcitool: Use install_url in the unattended install files guests: Add CentOS 8 mappings: Adjustments for CentOS 8 guests: Install EPEL repo on all CentOS guests guests: Enable PowerTools repo on CentOS 8 guests guests: Add projects to CentOS 8 guests: Build projects on CentOS 8 guests/configs/kickstart.cfg | 6 + guests/host_vars/libvirt-centos-8/docker.yml | 2 ++ guests/host_vars/libvirt-centos-8/install.yml | 2 ++ guests/host_vars/libvirt-centos-8/main.yml| 22 guests/inventory | 1 + guests/lcitool| 26 ++- guests/playbooks/build/jobs/defaults.yml | 2 ++ guests/playbooks/build/projects/libosinfo.yml | 1 + .../playbooks/build/projects/libvirt-dbus.yml | 2 ++ guests/playbooks/build/projects/libvirt.yml | 1 + .../build/projects/osinfo-db-tools.yml| 1 + .../playbooks/build/projects/virt-manager.yml | 3 +++ guests/playbooks/update/tasks/base.yml| 9 ++- guests/vars/mappings.yml | 4 +++ jenkins/jobs/defaults.yaml| 2 ++ jenkins/projects/libosinfo.yaml | 1 + jenkins/projects/libvirt-dbus.yaml| 2 ++ jenkins/projects/libvirt.yaml | 1 + jenkins/projects/osinfo-db-tools.yaml | 1 + jenkins/projects/virt-manager.yaml| 3 +++ 20 files changed, 90 insertions(+), 2 deletions(-) create mode 100644 guests/host_vars/libvirt-centos-8/docker.yml create mode 100644 guests/host_vars/libvirt-centos-8/install.yml create mode 100644 guests/host_vars/libvirt-centos-8/main.yml -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH v2 3/8] guests: Add CentOS 8
This commit allows calling `lcitool install libvirt-centos-8`. `lcitool update libvirt-centos-8 $project` does *not* work as: - EPEL repository is only enabled for CentOS 7; - The reason EPEL is needed because of some base packages (as screen); - There's no task to enable PowerTools repository; - PowerTools is needed because the development packages are distributed via this repo; - There's *no* project supported; Signed-off-by: Fabiano Fidêncio --- guests/host_vars/libvirt-centos-8/docker.yml | 2 ++ guests/host_vars/libvirt-centos-8/install.yml | 2 ++ guests/host_vars/libvirt-centos-8/main.yml| 7 +++ guests/inventory | 1 + 4 files changed, 12 insertions(+) create mode 100644 guests/host_vars/libvirt-centos-8/docker.yml create mode 100644 guests/host_vars/libvirt-centos-8/install.yml create mode 100644 guests/host_vars/libvirt-centos-8/main.yml diff --git a/guests/host_vars/libvirt-centos-8/docker.yml b/guests/host_vars/libvirt-centos-8/docker.yml new file mode 100644 index 000..10c2a50 --- /dev/null +++ b/guests/host_vars/libvirt-centos-8/docker.yml @@ -0,0 +1,2 @@ +--- +docker_base: centos:centos8 diff --git a/guests/host_vars/libvirt-centos-8/install.yml b/guests/host_vars/libvirt-centos-8/install.yml new file mode 100644 index 000..46facc1 --- /dev/null +++ b/guests/host_vars/libvirt-centos-8/install.yml @@ -0,0 +1,2 @@ +--- +install_url: http://mirror.centos.org/centos-8/8/BaseOS/x86_64/os/ diff --git a/guests/host_vars/libvirt-centos-8/main.yml b/guests/host_vars/libvirt-centos-8/main.yml new file mode 100644 index 000..a147183 --- /dev/null +++ b/guests/host_vars/libvirt-centos-8/main.yml @@ -0,0 +1,7 @@ +--- +package_format: 'rpm' +package_manager: 'dnf' +os_name: 'CentOS' +os_version: '8' + +ansible_python_interpreter: /usr/bin/python3 diff --git a/guests/inventory b/guests/inventory index ecdcc34..f062310 100644 --- a/guests/inventory +++ b/guests/inventory @@ -1,4 +1,5 @@ libvirt-centos-7 +libvirt-centos-8 libvirt-debian-9 libvirt-debian-10 libvirt-debian-sid -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH v2 2/8] lcitool: Use install_url in the unattended install files
Passing `url --url={{ install_url }}` to a kickstart file is, according
to Red Hat official documentation[0], mandatory for RHEL 8 and onwards
(which includes CentOS 8).
Using it for RHEL 7, CentOS 7, and Fedora does not cause any issue.
[0]:
https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/8/html/performing_an_advanced_rhel_installation/kickstart-commands-and-options-reference_installing-rhel-as-an-experienced-user
Signed-off-by: Fabiano Fidêncio
Reviewed-by: Andrea Bolognani
---
guests/configs/kickstart.cfg | 6 ++
guests/lcitool | 9 +
2 files changed, 15 insertions(+)
diff --git a/guests/configs/kickstart.cfg b/guests/configs/kickstart.cfg
index 713557a..76b7b65 100644
--- a/guests/configs/kickstart.cfg
+++ b/guests/configs/kickstart.cfg
@@ -1,3 +1,9 @@
+# Installation source
+#
+# The operating system will be installed from the following URL
+url --url={{ install_url }}
+
+
# Installer configuration
#
# Perform a text based installation followed by a reboot, and disable
diff --git a/guests/lcitool b/guests/lcitool
index 379fecc..803477e 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -542,9 +542,18 @@ class Application:
# Unattended install scripts are being generated on the fly, based
# on the templates present in guests/configs/
+unattended_options = [
+"install_url",
+]
+
initrd_template = os.path.join(base, "configs", install_config)
with open(initrd_template, 'r') as template:
content = template.read()
+for option in unattended_options:
+content = content.replace(
+"{{ " + option + " }}",
+facts[option]
+)
tempdir = tempfile.mkdtemp()
initrd_inject = os.path.join(tempdir, install_config)
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH v2 6/8] guests: Enable PowerTools repo on CentOS 8 guests
This is needed as all the development packages will come from the
PowerTools repository, which is not enabled by default.
Signed-off-by: Fabiano Fidêncio
Reviewed-by: Andrea Bolognani
---
guests/playbooks/update/tasks/base.yml | 8
1 file changed, 8 insertions(+)
diff --git a/guests/playbooks/update/tasks/base.yml
b/guests/playbooks/update/tasks/base.yml
index 965e442..b3ab738 100644
--- a/guests/playbooks/update/tasks/base.yml
+++ b/guests/playbooks/update/tasks/base.yml
@@ -9,6 +9,14 @@
- os_name == 'Fedora'
- os_version == 'Rawhide'
+- name: Enable PowerTools repository
+ command: '{{ package_manager }} config-manager --set-enabled PowerTools -y'
+ args:
+warn: no
+ when:
+- os_name == 'CentOS'
+- os_version == '8'
+
- name: Enable EPEL repository
package:
name: epel-release
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH v2 5/8] guests: Install EPEL repo on all CentOS guests
EPEL repository should be enabled on all supported CentOS guests as it brings in packages which are part of the "base packages" for libvirt-jenkins-ci. Signed-off-by: Fabiano Fidêncio Reviewed-by: Andrea Bolognani --- guests/playbooks/update/tasks/base.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/guests/playbooks/update/tasks/base.yml b/guests/playbooks/update/tasks/base.yml index 5153917..965e442 100644 --- a/guests/playbooks/update/tasks/base.yml +++ b/guests/playbooks/update/tasks/base.yml @@ -15,7 +15,6 @@ state: latest when: - os_name == 'CentOS' -- os_version == '7' - name: Update installed packages package: -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH v2 7/8] guests: Add projects to CentOS 8
Signed-off-by: Fabiano Fidêncio --- guests/host_vars/libvirt-centos-8/main.yml | 15 +++ 1 file changed, 15 insertions(+) diff --git a/guests/host_vars/libvirt-centos-8/main.yml b/guests/host_vars/libvirt-centos-8/main.yml index a147183..aae2313 100644 --- a/guests/host_vars/libvirt-centos-8/main.yml +++ b/guests/host_vars/libvirt-centos-8/main.yml @@ -1,4 +1,19 @@ --- +projects: + - libosinfo + - libvirt + - libvirt-dbus + - libvirt-glib + - libvirt-go + - libvirt-go-xml + - libvirt-ocaml + - libvirt-perl + - libvirt-python + - osinfo-db + - osinfo-db-tools + - virt-manager + - virt-viewer + package_format: 'rpm' package_manager: 'dnf' os_name: 'CentOS' -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virkeyfile: fix compilation error with clang
On 12/10/19 10:03 AM, Pavel Hrdina wrote:
> On Tue, Dec 10, 2019 at 03:57:59PM +0100, Ján Tomko wrote:
>> On Tue, Dec 10, 2019 at 03:29:43PM +0100, Pavel Hrdina wrote:
>>> On Tue, Dec 10, 2019 at 03:14:40PM +0100, Christophe de Dinechin wrote:
> On 10 Dec 2019, at 15:11, Pavel Hrdina wrote:
>
> Clang complains about condition being always true:
>
> src/util/virkeyfile.c:113:23: error: result of comparison of constant 128
> with expression of type 'const char' is always true
> [-Werror,-Wtautological-constant-out-of-range-compare]
>while (!IS_EOF && IS_ASCII(CUR) && CUR != ']')
> ^
> src/util/virkeyfile.c:80:26: note: expanded from macro 'IS_ASCII'
> ~~~ ^ ~~~
>
> Signed-off-by: Pavel Hrdina
> ---
> src/util/virkeyfile.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/util/virkeyfile.c b/src/util/virkeyfile.c
> index 816bfae96d..ee29bd7aa6 100644
> --- a/src/util/virkeyfile.c
> +++ b/src/util/virkeyfile.c
> @@ -77,7 +77,7 @@ struct _virKeyFileParserCtxt {
> #define IS_EOF (ctxt->cur >= ctxt->end)
> #define IS_EOL(c) (((c) == '\n') || ((c) == '\r'))
> #define IS_BLANK(c) (((c) == ' ') || ((c) == '\t'))
> -#define IS_ASCII(c) ((c) < 128)
> +#define IS_ASCII(c) (((unsigned char) c) < 128)
Probably want parentheses around c.
>>>
>>> Right, I'll fix it before pushing.
>>>
>>
>> with that change:
>> Reviewed-by: Ján Tomko
>
> Thanks, pushed now.
>
>> But the pre-existingcode seems odd - this is used to parse the INI-like
>> syntax
>> files for auth: https://libvirt.org/auth.html
>>
>> It does not really make sense to allow form feed or del in the group
>> entry name. I guess the only remotely reasonable character we'd block
>> by switching to g_ascii_isprint here is '\t'
>
> I was considering to "fix" the code as well but when looking into what
> GLib offers we can replace this whole file with GKeyFile [1].
>
> Pavel
>
> [1]
Ah cool. It would be nice to track a list somewhere of glib things that
can replace internal libvirt infrastructure. Maybe a subsection of the
bitesizedtasks wiki even though some of them will be not so bitesized.
What are the other glib bits people know about?
- Cole
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 0/1] jenkins: Start building on Ubuntu
On Tue, 2019-12-10 at 14:54 +, Daniel P. Berrangé wrote: > On Tue, Dec 10, 2019 at 02:54:22PM +0100, Andrea Bolognani wrote: > > The only reason why I'm even questioning whether this should be done > > is capacity for the hypervisor host: the machine we're running all > > builders on has > > > > CPUs: 8 > > Memory: 32 GiB > > Storage: 450 GiB > > > > and each of the guests is configured to use > > > > CPUs: 2 > > Memory: 2 GiB > > Storage: 20 GiB > > > > So while we're good, and actually have plenty of room to grow, on > > the memory and storage front, we're already overcommitting our CPUs > > pretty significantly, which I guess is at least part of the reason > > why builds take so long. > > NB the memory that's free is not really free - it is being usefull > as I/O cache for the VM disks. So more VMs will reduce I/O cache. > Whether that will actually impact us I don't know though. Oh yeah, I'm aware of that. But like you, I'm unsure about the exact impact that makes. We could arguably reduce the amount of memory assigned to guests to 1 GiB: building libvirt and friends is not exactly memory-intensive, and we could potentially benefit from the additionaly I/O cache or lessen the blow caused by adding more VMs. > More importantly though, AFAICT, those are not 8 real CPUs. > > virsh nodeinfo reports 8 cores, but virsh capabilities > reports it as a 1 socket, 4 core, 2 thread CPU. > > IOW we haven't really got 8 CPUs, more like equivalent of 5 CPUs. > as HT only really gives a x1.3 boost in best case, and I suspect > builds are not likely to be hitting the best case. That's also true. > > Can we afford to add 50% more load on the machine without making it > > unusable? I don't know. But I think it would be worthwhile to at > > least try and see how it handles an additional 25%, which is exactly > > what this series does. > > Giving it a try is ok I guess. Can I have an R-b then? O:-) > I expect there's probably more we can do to optimize the setup > too. > > For example, what actual features of qcow2 are we using ? We're > not snapshotting VMs, we don't need grow-on-demand allocation. > AFACT we're paying the performance cost of qcow2 (l1/l2 table > lookups & metadata caching), for no reason. Switch the VMs to > fully pre-allocated raw files may improve I/O performance. > Raw LVM VGs would be even better but that will be painful > to setup given the host install setup. > > I also wonder if we have the optimal aio setting for disks, > as there's nothing in the XML. > > We could consider using cache=unsafe for VMs, though for > that I think we'd want to separate off a separate disk > for /home/jenkins so that if there was a host OS crash, > we wouldn't have to rebuild the entire VMs - just throw > away the data disk & recreate. The home directory for the jenkins user contains some configuration as well, so you'd have to run 'lcitool update' anyway after attaching the new disk... At that point, it might be less work overall to just rebuild the entire VM from scratch. We have only seen a couple of unexpected host shutdowns so far, and all were caused by hardware issues that resulted in a multi-day downtime anyway, so the overhead of reinstalling the various guest OS' would not be the dealbreaker. > Since we've got plenty of RAM, another obvious thing would be > to turn on huge pages and use them for all guest RAM. This may > well have a very significant performance boost from reducing > CPU overhead which is our biggest bottleneck. Yeah, all of the above sounds like it could help, but I'm not really well versed on the performance tuning front so I wouldn't know where to start and even how to properly measure the impact of each change. We also have to keep in mind that, while the CentOS CI environment is the main consumer of the repository, we want it to be possible for any developer to deploy builders locally relatively painlessly, so things like hugepages and LVM usage would definitely have to be optional if we introduced them. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] doc: vtpm only support secrets by UUID at this point
From: Marc-André Lureau Support by usage name can be considered separately (with a 'usage' attribute?). Cc: Stefan Berger Signed-off-by: Marc-André Lureau --- docs/formatsecret.html.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/formatsecret.html.in b/docs/formatsecret.html.in index 8d0630a7c3..8f5383cf64 100644 --- a/docs/formatsecret.html.in +++ b/docs/formatsecret.html.in @@ -334,8 +334,8 @@ Secret value set of the vTPM. The usage type='vtpm' element must contain a single name element that specifies a usage name - for the secret. The vTPM secret can then be used by UUID or by - this usage name via the encryption element of + for the secret. The vTPM secret can then be used by UUID + via the encryption element of a tpm when using an emulator. Since 5.6.0. The following is an example -- 2.24.0.308.g228f53135a -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virkeyfile: fix compilation error with clang
On Tue, Dec 10, 2019 at 04:03:56PM +0100, Pavel Hrdina wrote:
> On Tue, Dec 10, 2019 at 03:57:59PM +0100, Ján Tomko wrote:
> > On Tue, Dec 10, 2019 at 03:29:43PM +0100, Pavel Hrdina wrote:
> > > On Tue, Dec 10, 2019 at 03:14:40PM +0100, Christophe de Dinechin wrote:
> > > >
> > > >
> > > > > On 10 Dec 2019, at 15:11, Pavel Hrdina wrote:
> > > > >
> > > > > Clang complains about condition being always true:
> > > > >
> > > > > src/util/virkeyfile.c:113:23: error: result of comparison of constant
> > > > > 128 with expression of type 'const char' is always true
> > > > > [-Werror,-Wtautological-constant-out-of-range-compare]
> > > > >while (!IS_EOF && IS_ASCII(CUR) && CUR != ']')
> > > > > ^
> > > > > src/util/virkeyfile.c:80:26: note: expanded from macro 'IS_ASCII'
> > > > > ~~~ ^ ~~~
> > > > >
> > > > > Signed-off-by: Pavel Hrdina
> > > > > ---
> > > > > src/util/virkeyfile.c | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/src/util/virkeyfile.c b/src/util/virkeyfile.c
> > > > > index 816bfae96d..ee29bd7aa6 100644
> > > > > --- a/src/util/virkeyfile.c
> > > > > +++ b/src/util/virkeyfile.c
> > > > > @@ -77,7 +77,7 @@ struct _virKeyFileParserCtxt {
> > > > > #define IS_EOF (ctxt->cur >= ctxt->end)
> > > > > #define IS_EOL(c) (((c) == '\n') || ((c) == '\r'))
> > > > > #define IS_BLANK(c) (((c) == ' ') || ((c) == '\t'))
> > > > > -#define IS_ASCII(c) ((c) < 128)
> > > > > +#define IS_ASCII(c) (((unsigned char) c) < 128)
> > > >
> > > > Probably want parentheses around c.
> > >
> > > Right, I'll fix it before pushing.
> > >
> >
> > with that change:
> > Reviewed-by: Ján Tomko
>
> Thanks, pushed now.
>
> > But the pre-existingcode seems odd - this is used to parse the INI-like
> > syntax
> > files for auth: https://libvirt.org/auth.html
> >
> > It does not really make sense to allow form feed or del in the group
> > entry name. I guess the only remotely reasonable character we'd block
> > by switching to g_ascii_isprint here is '\t'
>
> I was considering to "fix" the code as well but when looking into what
> GLib offers we can replace this whole file with GKeyFile [1].
Yes, if GKeyFile is compatible with what we're currently parsing,
we should definitely just replace it
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] virkeyfile: fix compilation error with clang
On Tue, Dec 10, 2019 at 03:57:59PM +0100, Ján Tomko wrote:
> On Tue, Dec 10, 2019 at 03:29:43PM +0100, Pavel Hrdina wrote:
> > On Tue, Dec 10, 2019 at 03:14:40PM +0100, Christophe de Dinechin wrote:
> > >
> > >
> > > > On 10 Dec 2019, at 15:11, Pavel Hrdina wrote:
> > > >
> > > > Clang complains about condition being always true:
> > > >
> > > > src/util/virkeyfile.c:113:23: error: result of comparison of constant
> > > > 128 with expression of type 'const char' is always true
> > > > [-Werror,-Wtautological-constant-out-of-range-compare]
> > > >while (!IS_EOF && IS_ASCII(CUR) && CUR != ']')
> > > > ^
> > > > src/util/virkeyfile.c:80:26: note: expanded from macro 'IS_ASCII'
> > > > ~~~ ^ ~~~
> > > >
> > > > Signed-off-by: Pavel Hrdina
> > > > ---
> > > > src/util/virkeyfile.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/src/util/virkeyfile.c b/src/util/virkeyfile.c
> > > > index 816bfae96d..ee29bd7aa6 100644
> > > > --- a/src/util/virkeyfile.c
> > > > +++ b/src/util/virkeyfile.c
> > > > @@ -77,7 +77,7 @@ struct _virKeyFileParserCtxt {
> > > > #define IS_EOF (ctxt->cur >= ctxt->end)
> > > > #define IS_EOL(c) (((c) == '\n') || ((c) == '\r'))
> > > > #define IS_BLANK(c) (((c) == ' ') || ((c) == '\t'))
> > > > -#define IS_ASCII(c) ((c) < 128)
> > > > +#define IS_ASCII(c) (((unsigned char) c) < 128)
> > >
> > > Probably want parentheses around c.
> >
> > Right, I'll fix it before pushing.
> >
>
> with that change:
> Reviewed-by: Ján Tomko
Thanks, pushed now.
> But the pre-existingcode seems odd - this is used to parse the INI-like syntax
> files for auth: https://libvirt.org/auth.html
>
> It does not really make sense to allow form feed or del in the group
> entry name. I guess the only remotely reasonable character we'd block
> by switching to g_ascii_isprint here is '\t'
I was considering to "fix" the code as well but when looking into what
GLib offers we can replace this whole file with GKeyFile [1].
Pavel
[1]
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virkeyfile: fix compilation error with clang
On Tue, Dec 10, 2019 at 03:29:43PM +0100, Pavel Hrdina wrote:
On Tue, Dec 10, 2019 at 03:14:40PM +0100, Christophe de Dinechin wrote:
> On 10 Dec 2019, at 15:11, Pavel Hrdina wrote:
>
> Clang complains about condition being always true:
>
> src/util/virkeyfile.c:113:23: error: result of comparison of constant 128
with expression of type 'const char' is always true
[-Werror,-Wtautological-constant-out-of-range-compare]
>while (!IS_EOF && IS_ASCII(CUR) && CUR != ']')
> ^
> src/util/virkeyfile.c:80:26: note: expanded from macro 'IS_ASCII'
> ~~~ ^ ~~~
>
> Signed-off-by: Pavel Hrdina
> ---
> src/util/virkeyfile.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/util/virkeyfile.c b/src/util/virkeyfile.c
> index 816bfae96d..ee29bd7aa6 100644
> --- a/src/util/virkeyfile.c
> +++ b/src/util/virkeyfile.c
> @@ -77,7 +77,7 @@ struct _virKeyFileParserCtxt {
> #define IS_EOF (ctxt->cur >= ctxt->end)
> #define IS_EOL(c) (((c) == '\n') || ((c) == '\r'))
> #define IS_BLANK(c) (((c) == ' ') || ((c) == '\t'))
> -#define IS_ASCII(c) ((c) < 128)
> +#define IS_ASCII(c) (((unsigned char) c) < 128)
Probably want parentheses around c.
Right, I'll fix it before pushing.
with that change:
Reviewed-by: Ján Tomko
But the pre-existingcode seems odd - this is used to parse the INI-like syntax
files for auth: https://libvirt.org/auth.html
It does not really make sense to allow form feed or del in the group
entry name. I guess the only remotely reasonable character we'd block
by switching to g_ascii_isprint here is '\t'
Jano
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 0/1] jenkins: Start building on Ubuntu
On Tue, Dec 10, 2019 at 02:54:22PM +0100, Andrea Bolognani wrote: > This patch is intended to start a slightly larger discussion about > our plans for the CentOS CI environment going forward. > > At the moment, we have active builders for > > CentOS 7 > Debian 9 > Debian 10 > Fedora 30 > Fedora 31 > Fedora Rawhide > FreeBSD 11 > FreeBSD 12 > > but we don't have builder for > > Debian sid > FreeBSD -CURRENT > Ubuntu 16.04 > Ubuntu 18.04 > > despite them being fully supported in the libvirt-jenkins-ci > repository. > > This makes sense for sid and -CURRENT, since the former covers the > same "freshest Linux packages" angle that Rawhide already takes care > of and the latter is often broken and not trivial to keep updated; > both Ubuntu targets, however, should IMHO be part of the CentOS CI > environment. Hence this series :) > > Moreover, we're in the process of adding > > CentOS 8 > openSUSE Leap 15.1 > openSUSE Tumbleweed > > as targets, of which the first two should also IMHO be added as they > would provide useful additional coverage. > > The only reason why I'm even questioning whether this should be done > is capacity for the hypervisor host: the machine we're running all > builders on has > > CPUs: 8 > Memory: 32 GiB > Storage: 450 GiB > > and each of the guests is configured to use > > CPUs: 2 > Memory: 2 GiB > Storage: 20 GiB > > So while we're good, and actually have plenty of room to grow, on > the memory and storage front, we're already overcommitting our CPUs > pretty significantly, which I guess is at least part of the reason > why builds take so long. NB the memory that's free is not really free - it is being usefull as I/O cache for the VM disks. So more VMs will reduce I/O cache. Whether that will actually impact us I don't know though. More importantly though, AFAICT, those are not 8 real CPUs. virsh nodeinfo reports 8 cores, but virsh capabilities reports it as a 1 socket, 4 core, 2 thread CPU. IOW we haven't really got 8 CPUs, more like equivalent of 5 CPUs. as HT only really gives a x1.3 boost in best case, and I suspect builds are not likely to be hitting the best case. > Can we afford to add 50% more load on the machine without making it > unusable? I don't know. But I think it would be worthwhile to at > least try and see how it handles an additional 25%, which is exactly > what this series does. Giving it a try is ok I guess. I expect there's probably more we can do to optimize the setup too. For example, what actual features of qcow2 are we using ? We're not snapshotting VMs, we don't need grow-on-demand allocation. AFACT we're paying the performance cost of qcow2 (l1/l2 table lookups & metadata caching), for no reason. Switch the VMs to fully pre-allocated raw files may improve I/O performance. Raw LVM VGs would be even better but that will be painful to setup given the host install setup. I also wonder if we have the optimal aio setting for disks, as there's nothing in the XML. We could consider using cache=unsafe for VMs, though for that I think we'd want to separate off a separate disk for /home/jenkins so that if there was a host OS crash, we wouldn't have to rebuild the entire VMs - just throw away the data disk & recreate. Since we've got plenty of RAM, another obvious thing would be to turn on huge pages and use them for all guest RAM. This may well have a very significant performance boost from reducing CPU overhead which is our biggest bottleneck. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/1] qemu: fix crash bug in snapshot revert
This is a significant rework of v1 as - patch 1 was dropped because an equivalent change was accidentally committed in the meantime in 4c53267b70fc5c548b6530113c3f96870d8d7fc1 - patch 2 was dropped as it was just a mostly formal clean-up of patch 1 - patch 3 was redone using a different approach. Patch 3 now takes the approach that was hinted at in the last paragraph of v1's cover letter, more specifically it now fixes qemuProcessStop() rather than its caller. Digging into git history I located the commit that introduced qemuProcessStop()'s problematic behaviour and after consulting with Michal, we agreed that fixing qemuProcessStop() seems safe enough, and definitely a better solution conceptually. Pavel Mores (1): qemu: fix concurrency crash bug in snapshot revert src/qemu/qemu_domain.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/1] qemu: fix concurrency crash bug in snapshot revert
This commit aims to fix
https://bugzilla.redhat.com/show_bug.cgi?id=1610207
The cause was apparently incorrect handling of jobs in snapshot revert code
which allowed a thread executing snapshot delete to begin job while
snapshot revert was still running on another thread. The snapshot delete
thread then waited on a condition variable in qemuMonitorSend() while the
revert thread finished, changing (and effectively corrupting) the
qemuMonitor structure under the delete thread which led to its crash.
The incorrect handling of jobs in revert code was due to the fact that
although qemuDomainRevertToSnapshot() correctly begins a job at the start,
the job was implicitly ended when qemuProcessStop() was called because the
job lives in the QEMU driver's private data (qemuDomainObjPrivate) that
was purged during qemuProcessStop().
This fix prevents qemuProcessStop() from clearing jobs as the idea of
qemuProcessStop() clearing jobs seems wrong in the first place. It was
(inadvertently) introduced in commit 888aa4b6b9db65e3db273341e79846, which
is effectively reverted by the second hunk of this commit. To preserve
the desired effects of the faulty commit, the first hunk is included as
suggested by Michal.
Signed-off-by: Pavel Mores
---
src/qemu/qemu_domain.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 6f53e17b6a..e4a1bccc18 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -403,6 +403,8 @@ qemuDomainObjRestoreJob(virDomainObjPtr obj,
static void
qemuDomainObjFreeJob(qemuDomainObjPrivatePtr priv)
{
+qemuDomainObjResetJob(priv);
+qemuDomainObjResetAsyncJob(priv);
VIR_FREE(priv->job.current);
VIR_FREE(priv->job.completed);
virCondDestroy(>job.cond);
@@ -2161,9 +2163,6 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr
priv)
virBitmapFree(priv->migrationCaps);
priv->migrationCaps = NULL;
-qemuDomainObjResetJob(priv);
-qemuDomainObjResetAsyncJob(priv);
-
virHashRemoveAll(priv->blockjobs);
virHashRemoveAll(priv->dbusVMStates);
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virkeyfile: fix compilation error with clang
On Tue, Dec 10, 2019 at 03:14:40PM +0100, Christophe de Dinechin wrote:
>
>
> > On 10 Dec 2019, at 15:11, Pavel Hrdina wrote:
> >
> > Clang complains about condition being always true:
> >
> > src/util/virkeyfile.c:113:23: error: result of comparison of constant 128
> > with expression of type 'const char' is always true
> > [-Werror,-Wtautological-constant-out-of-range-compare]
> >while (!IS_EOF && IS_ASCII(CUR) && CUR != ']')
> > ^
> > src/util/virkeyfile.c:80:26: note: expanded from macro 'IS_ASCII'
> > ~~~ ^ ~~~
> >
> > Signed-off-by: Pavel Hrdina
> > ---
> > src/util/virkeyfile.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/util/virkeyfile.c b/src/util/virkeyfile.c
> > index 816bfae96d..ee29bd7aa6 100644
> > --- a/src/util/virkeyfile.c
> > +++ b/src/util/virkeyfile.c
> > @@ -77,7 +77,7 @@ struct _virKeyFileParserCtxt {
> > #define IS_EOF (ctxt->cur >= ctxt->end)
> > #define IS_EOL(c) (((c) == '\n') || ((c) == '\r'))
> > #define IS_BLANK(c) (((c) == ' ') || ((c) == '\t'))
> > -#define IS_ASCII(c) ((c) < 128)
> > +#define IS_ASCII(c) (((unsigned char) c) < 128)
>
> Probably want parentheses around c.
Right, I'll fix it before pushing.
>
>
> > #define CUR (*ctxt->cur)
> > #define NEXT if (!IS_EOF) ctxt->cur++;
> >
> > --
> > 2.23.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
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virkeyfile: fix compilation error with clang
> On 10 Dec 2019, at 15:11, Pavel Hrdina wrote:
>
> Clang complains about condition being always true:
>
> src/util/virkeyfile.c:113:23: error: result of comparison of constant 128
> with expression of type 'const char' is always true
> [-Werror,-Wtautological-constant-out-of-range-compare]
>while (!IS_EOF && IS_ASCII(CUR) && CUR != ']')
> ^
> src/util/virkeyfile.c:80:26: note: expanded from macro 'IS_ASCII'
> ~~~ ^ ~~~
>
> Signed-off-by: Pavel Hrdina
> ---
> src/util/virkeyfile.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/util/virkeyfile.c b/src/util/virkeyfile.c
> index 816bfae96d..ee29bd7aa6 100644
> --- a/src/util/virkeyfile.c
> +++ b/src/util/virkeyfile.c
> @@ -77,7 +77,7 @@ struct _virKeyFileParserCtxt {
> #define IS_EOF (ctxt->cur >= ctxt->end)
> #define IS_EOL(c) (((c) == '\n') || ((c) == '\r'))
> #define IS_BLANK(c) (((c) == ' ') || ((c) == '\t'))
> -#define IS_ASCII(c) ((c) < 128)
> +#define IS_ASCII(c) (((unsigned char) c) < 128)
Probably want parentheses around c.
> #define CUR (*ctxt->cur)
> #define NEXT if (!IS_EOF) ctxt->cur++;
>
> --
> 2.23.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] [perl PATCH 0/6] Misc fixes to perl APIs and new migrate constant
On 12/10/19 2:17 PM, Daniel P. Berrangé wrote: Daniel P. Berrangé (6): Fix typo breaking some migrate parameter handling Add handling for VIR_MIGRATE_PARAM_PARALLEL_CONNECTIONS Add support for VIR_MIGRATE_PARAM_TLS_DESTINATION constant Add missing create_checkpoint method Remove docs for has_metadata method which doesn't exist Fix data type for VIR_CONNECT_IDENTITY_SASL_USER_NAME Changes | 9 + lib/Sys/Virt.xs | 25 + lib/Sys/Virt/Domain.pm | 26 ++ lib/Sys/Virt/DomainCheckpoint.pm | 5 - 4 files changed, 56 insertions(+), 9 deletions(-) Reviewed-by: Michal Privoznik Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virkeyfile: fix compilation error with clang
Clang complains about condition being always true:
src/util/virkeyfile.c:113:23: error: result of comparison of constant 128 with
expression of type 'const char' is always true
[-Werror,-Wtautological-constant-out-of-range-compare]
while (!IS_EOF && IS_ASCII(CUR) && CUR != ']')
^
src/util/virkeyfile.c:80:26: note: expanded from macro 'IS_ASCII'
~~~ ^ ~~~
Signed-off-by: Pavel Hrdina
---
src/util/virkeyfile.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virkeyfile.c b/src/util/virkeyfile.c
index 816bfae96d..ee29bd7aa6 100644
--- a/src/util/virkeyfile.c
+++ b/src/util/virkeyfile.c
@@ -77,7 +77,7 @@ struct _virKeyFileParserCtxt {
#define IS_EOF (ctxt->cur >= ctxt->end)
#define IS_EOL(c) (((c) == '\n') || ((c) == '\r'))
#define IS_BLANK(c) (((c) == ' ') || ((c) == '\t'))
-#define IS_ASCII(c) ((c) < 128)
+#define IS_ASCII(c) (((unsigned char) c) < 128)
#define CUR (*ctxt->cur)
#define NEXT if (!IS_EOF) ctxt->cur++;
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH 1/1] jenkins: Start building on Ubuntu
We've supported both Ubuntu 16.04 and Ubuntu 18.04 as build targets
for a long time now, and we are even building on the latter on
Travis CI, but we've never actually deployed the corresponding VMs
in the CentOS CI environment. Start doing that now.
Signed-off-by: Andrea Bolognani
---
jenkins/jobs/defaults.yaml| 2 ++
jenkins/projects/libvirt-dbus.yaml| 1 +
jenkins/projects/libvirt-sandbox.yaml | 2 ++
jenkins/projects/libvirt-tck.yaml | 2 ++
jenkins/projects/libvirt.yaml | 2 ++
jenkins/projects/virt-manager.yaml| 2 ++
6 files changed, 11 insertions(+)
diff --git a/jenkins/jobs/defaults.yaml b/jenkins/jobs/defaults.yaml
index 676ecbf..18bbade 100644
--- a/jenkins/jobs/defaults.yaml
+++ b/jenkins/jobs/defaults.yaml
@@ -11,6 +11,8 @@
- libvirt-fedora-rawhide
- libvirt-freebsd-11
- libvirt-freebsd-12
+ - libvirt-ubuntu-1604
+ - libvirt-ubuntu-1804
rpm_machines:
- libvirt-centos-7
- libvirt-fedora-30
diff --git a/jenkins/projects/libvirt-dbus.yaml
b/jenkins/projects/libvirt-dbus.yaml
index dfc159c..db9d64b 100644
--- a/jenkins/projects/libvirt-dbus.yaml
+++ b/jenkins/projects/libvirt-dbus.yaml
@@ -20,6 +20,7 @@
- libvirt-fedora-30
- libvirt-fedora-31
- libvirt-fedora-rawhide
+- libvirt-ubuntu-1804
- meson-rpm-job:
parent_jobs: 'libvirt-dbus-check'
# RPM build is still not possible on CentOS7 as it does not
diff --git a/jenkins/projects/libvirt-sandbox.yaml
b/jenkins/projects/libvirt-sandbox.yaml
index 6a8acec..00ecb98 100644
--- a/jenkins/projects/libvirt-sandbox.yaml
+++ b/jenkins/projects/libvirt-sandbox.yaml
@@ -10,6 +10,8 @@
- libvirt-fedora-30
- libvirt-fedora-31
- libvirt-fedora-rawhide
+ - libvirt-ubuntu-1604
+ - libvirt-ubuntu-1804
title: Libvirt Sandbox
archive_format: gz
git_url: '{git_urls[libvirt-sandbox][default]}'
diff --git a/jenkins/projects/libvirt-tck.yaml
b/jenkins/projects/libvirt-tck.yaml
index fcdea98..4e129ee 100644
--- a/jenkins/projects/libvirt-tck.yaml
+++ b/jenkins/projects/libvirt-tck.yaml
@@ -11,6 +11,8 @@
- libvirt-fedora-rawhide
- libvirt-freebsd-11
- libvirt-freebsd-12
+ - libvirt-ubuntu-1604
+ - libvirt-ubuntu-1804
title: Libvirt TCK
archive_format: gz
git_url: '{git_urls[libvirt-tck][default]}'
diff --git a/jenkins/projects/libvirt.yaml b/jenkins/projects/libvirt.yaml
index fdc24bc..9c5922f 100644
--- a/jenkins/projects/libvirt.yaml
+++ b/jenkins/projects/libvirt.yaml
@@ -19,6 +19,8 @@
- libvirt-fedora-30
- libvirt-fedora-31
- libvirt-fedora-rawhide
+- libvirt-ubuntu-1604
+- libvirt-ubuntu-1804
- autotools-check-job:
parent_jobs: 'libvirt-syntax-check'
local_env: |
diff --git a/jenkins/projects/virt-manager.yaml
b/jenkins/projects/virt-manager.yaml
index 3dc8e2e..81bee13 100644
--- a/jenkins/projects/virt-manager.yaml
+++ b/jenkins/projects/virt-manager.yaml
@@ -12,6 +12,7 @@
- libvirt-fedora-rawhide
- libvirt-freebsd-11
- libvirt-freebsd-12
+ - libvirt-ubuntu-1804
title: Virtual Machine Manager
archive_format: gz
git_url: '{git_urls[virt-manager][default]}'
@@ -33,6 +34,7 @@
- libvirt-fedora-30
- libvirt-fedora-31
- libvirt-fedora-rawhide
+- libvirt-ubuntu-1804
- python-distutils-rpm-job:
parent_jobs: 'virt-manager-check'
machines:
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH 0/1] jenkins: Start building on Ubuntu
This patch is intended to start a slightly larger discussion about our plans for the CentOS CI environment going forward. At the moment, we have active builders for CentOS 7 Debian 9 Debian 10 Fedora 30 Fedora 31 Fedora Rawhide FreeBSD 11 FreeBSD 12 but we don't have builder for Debian sid FreeBSD -CURRENT Ubuntu 16.04 Ubuntu 18.04 despite them being fully supported in the libvirt-jenkins-ci repository. This makes sense for sid and -CURRENT, since the former covers the same "freshest Linux packages" angle that Rawhide already takes care of and the latter is often broken and not trivial to keep updated; both Ubuntu targets, however, should IMHO be part of the CentOS CI environment. Hence this series :) Moreover, we're in the process of adding CentOS 8 openSUSE Leap 15.1 openSUSE Tumbleweed as targets, of which the first two should also IMHO be added as they would provide useful additional coverage. The only reason why I'm even questioning whether this should be done is capacity for the hypervisor host: the machine we're running all builders on has CPUs: 8 Memory: 32 GiB Storage: 450 GiB and each of the guests is configured to use CPUs: 2 Memory: 2 GiB Storage: 20 GiB So while we're good, and actually have plenty of room to grow, on the memory and storage front, we're already overcommitting our CPUs pretty significantly, which I guess is at least part of the reason why builds take so long. Can we afford to add 50% more load on the machine without making it unusable? I don't know. But I think it would be worthwhile to at least try and see how it handles an additional 25%, which is exactly what this series does. In my opinion, as long as the machine can keep up with demand and not end up in a situation where it starts accumulating backlog jobs, it's fine if builds take longer: developers who want to run a relatively quick smoke test before posting patches can use Travis CI or 'make ci-check@...' locally for the purpose, whereas the role of CentOS CI in my eyes is to try and catch as many issues as possible after merge so that they don't end up in a release. But I realize others might see it differently, hence this lenghty cover letter :) Andrea Bolognani (1): jenkins: Start building on Ubuntu jenkins/jobs/defaults.yaml| 2 ++ jenkins/projects/libvirt-dbus.yaml| 1 + jenkins/projects/libvirt-sandbox.yaml | 2 ++ jenkins/projects/libvirt-tck.yaml | 2 ++ jenkins/projects/libvirt.yaml | 2 ++ jenkins/projects/virt-manager.yaml| 2 ++ 6 files changed, 11 insertions(+) -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [perl PATCH 5/6] Remove docs for has_metadata method which doesn't exist
Signed-off-by: Daniel P. Berrangé --- Changes | 1 + lib/Sys/Virt/DomainCheckpoint.pm | 5 - 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/Changes b/Changes index f36d401..9d9a5c8 100644 --- a/Changes +++ b/Changes @@ -10,6 +10,7 @@ Revision history for perl module Sys::Virt constant when invoking migrate - Add VIR_MIGRATE_PARAM_TLS_DESTINATION constant - Add missing create_checkpoint method on Sys::Virt::Domain + - Remove docs for has_metadata method which doesn't exit 5.10.0 2019-12-03 diff --git a/lib/Sys/Virt/DomainCheckpoint.pm b/lib/Sys/Virt/DomainCheckpoint.pm index e8e875d..e1adae4 100644 --- a/lib/Sys/Virt/DomainCheckpoint.pm +++ b/lib/Sys/Virt/DomainCheckpoint.pm @@ -97,11 +97,6 @@ constants. Return the parent of the checkpoint, if any -=item $res = $domchkp->has_metadata() - -Returns a true value if this checkpoint has metadata associated with -it. - =item my @checkpoints = $domchkp->list_all_children($flags) Return a list of all domain checkpoints that are children of this -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [perl PATCH 1/6] Fix typo breaking some migrate parameter handling
Signed-off-by: Daniel P. Berrangé --- Changes | 2 ++ lib/Sys/Virt.xs | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Changes b/Changes index 37529ac..2ba87bb 100644 --- a/Changes +++ b/Changes @@ -4,6 +4,8 @@ Revision history for perl module Sys::Virt - Add VIR_DOMAIN_JOB_SUCCESS and VIR_DOMAIN_JOB_STATS_KEEP_COMPLETED constants + - Fix typo breaking migration postcopy bandwidth and + autoconvert increment parameter handling 5.10.0 2019-12-03 diff --git a/lib/Sys/Virt.xs b/lib/Sys/Virt.xs index 98a12d7..08fd084 100644 --- a/lib/Sys/Virt.xs +++ b/lib/Sys/Virt.xs @@ -5571,7 +5571,7 @@ _migrate_to_uri(dom, desturi, newparams, flags=0) VIR_TYPED_PARAM_FIELD_LENGTH); params[14].type = VIR_TYPED_PARAM_INT; - strncpy(params[14].field, VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY, + strncpy(params[15].field, VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY, VIR_TYPED_PARAM_FIELD_LENGTH); params[15].type = VIR_TYPED_PARAM_ULLONG; -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [perl PATCH 2/6] Add handling for VIR_MIGRATE_PARAM_PARALLEL_CONNECTIONS
Signed-off-by: Daniel P. Berrangé --- Changes | 2 ++ lib/Sys/Virt.xs | 12 ++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/Changes b/Changes index 2ba87bb..ba33d40 100644 --- a/Changes +++ b/Changes @@ -6,6 +6,8 @@ Revision history for perl module Sys::Virt constants - Fix typo breaking migration postcopy bandwidth and autoconvert increment parameter handling + - Add handling for VIR_MIGRATE_PARAM_PARALLEL_CONNECTIONS + constant when invoking migrate 5.10.0 2019-12-03 diff --git a/lib/Sys/Virt.xs b/lib/Sys/Virt.xs index 08fd084..b52ae7d 100644 --- a/lib/Sys/Virt.xs +++ b/lib/Sys/Virt.xs @@ -5413,7 +5413,7 @@ _migrate(dom, destcon, newparams, flags=0) virTypedParameterPtr params; int nparams; CODE: - nparams = 16; + nparams = 17; Newx(params, nparams, virTypedParameter); strncpy(params[0].field, VIR_MIGRATE_PARAM_URI, @@ -5480,6 +5480,10 @@ _migrate(dom, destcon, newparams, flags=0) VIR_TYPED_PARAM_FIELD_LENGTH); params[15].type = VIR_TYPED_PARAM_ULLONG; + strncpy(params[16].field, VIR_MIGRATE_PARAM_PARALLEL_CONNECTIONS, + VIR_TYPED_PARAM_FIELD_LENGTH); + params[16].type = VIR_TYPED_PARAM_INT; + nparams = vir_typed_param_from_hv(newparams, params, nparams); vir_typed_param_add_string_list_from_hv(newparams, , , @@ -5508,7 +5512,7 @@ _migrate_to_uri(dom, desturi, newparams, flags=0) virTypedParameterPtr params; int nparams; PPCODE: - nparams = 16; + nparams = 17; Newx(params, nparams, virTypedParameter); strncpy(params[0].field, VIR_MIGRATE_PARAM_URI, @@ -5575,6 +5579,10 @@ _migrate_to_uri(dom, desturi, newparams, flags=0) VIR_TYPED_PARAM_FIELD_LENGTH); params[15].type = VIR_TYPED_PARAM_ULLONG; + strncpy(params[16].field, VIR_MIGRATE_PARAM_PARALLEL_CONNECTIONS, + VIR_TYPED_PARAM_FIELD_LENGTH); + params[16].type = VIR_TYPED_PARAM_INT; + nparams = vir_typed_param_from_hv(newparams, params, nparams); vir_typed_param_add_string_list_from_hv(newparams, , , -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [perl PATCH 3/6] Add support for VIR_MIGRATE_PARAM_TLS_DESTINATION constant
Signed-off-by: Daniel P. Berrangé --- Changes| 1 + lib/Sys/Virt.xs| 13 +++-- lib/Sys/Virt/Domain.pm | 9 + 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/Changes b/Changes index ba33d40..09d1dca 100644 --- a/Changes +++ b/Changes @@ -8,6 +8,7 @@ Revision history for perl module Sys::Virt autoconvert increment parameter handling - Add handling for VIR_MIGRATE_PARAM_PARALLEL_CONNECTIONS constant when invoking migrate + - Add VIR_MIGRATE_PARAM_TLS_DESTINATION constant 5.10.0 2019-12-03 diff --git a/lib/Sys/Virt.xs b/lib/Sys/Virt.xs index b52ae7d..3245adf 100644 --- a/lib/Sys/Virt.xs +++ b/lib/Sys/Virt.xs @@ -5413,7 +5413,7 @@ _migrate(dom, destcon, newparams, flags=0) virTypedParameterPtr params; int nparams; CODE: - nparams = 17; + nparams = 18; Newx(params, nparams, virTypedParameter); strncpy(params[0].field, VIR_MIGRATE_PARAM_URI, @@ -5484,6 +5484,10 @@ _migrate(dom, destcon, newparams, flags=0) VIR_TYPED_PARAM_FIELD_LENGTH); params[16].type = VIR_TYPED_PARAM_INT; + strncpy(params[17].field, VIR_MIGRATE_PARAM_TLS_DESTINATION, + VIR_TYPED_PARAM_FIELD_LENGTH); + params[17].type = VIR_TYPED_PARAM_STRING; + nparams = vir_typed_param_from_hv(newparams, params, nparams); vir_typed_param_add_string_list_from_hv(newparams, , , @@ -5512,7 +5516,7 @@ _migrate_to_uri(dom, desturi, newparams, flags=0) virTypedParameterPtr params; int nparams; PPCODE: - nparams = 17; + nparams = 18; Newx(params, nparams, virTypedParameter); strncpy(params[0].field, VIR_MIGRATE_PARAM_URI, @@ -5583,6 +5587,10 @@ _migrate_to_uri(dom, desturi, newparams, flags=0) VIR_TYPED_PARAM_FIELD_LENGTH); params[16].type = VIR_TYPED_PARAM_INT; + strncpy(params[17].field, VIR_MIGRATE_PARAM_TLS_DESTINATION, + VIR_TYPED_PARAM_FIELD_LENGTH); + params[17].type = VIR_TYPED_PARAM_STRING; + nparams = vir_typed_param_from_hv(newparams, params, nparams); vir_typed_param_add_string_list_from_hv(newparams, , , @@ -9254,6 +9262,7 @@ BOOT: REGISTER_CONSTANT_STR(VIR_MIGRATE_PARAM_AUTO_CONVERGE_INCREMENT, MIGRATE_PARAM_AUTO_CONVERGE_INCREMENT); REGISTER_CONSTANT_STR(VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY, MIGRATE_PARAM_BANDWIDTH_POSTCOPY); REGISTER_CONSTANT_STR(VIR_MIGRATE_PARAM_PARALLEL_CONNECTIONS, MIGRATE_PARAM_PARALLEL_CONNECTIONS); + REGISTER_CONSTANT_STR(VIR_MIGRATE_PARAM_TLS_DESTINATION, MIGRATE_PARAM_TLS_DESTINATION); REGISTER_CONSTANT(VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY, MIGRATE_MAX_SPEED_POSTCOPY); diff --git a/lib/Sys/Virt/Domain.pm b/lib/Sys/Virt/Domain.pm index 2caec51..ae909de 100644 --- a/lib/Sys/Virt/Domain.pm +++ b/lib/Sys/Virt/Domain.pm @@ -1141,6 +1141,15 @@ progress is not made The number of connections used during parallel migration. +=item C + +Override the destination host name used for TLS verification. +Normally the TLS certificate from the destination host must match +the host's name for TLS verification to succeed. When the +certificate does not match the destination hostname and the +expected cetificate's hostname is known, this parameter can be +used to pass this expected hostname when starting the migration. + =back =item $ddom = $dom->migrate(destcon, flags=0, dname=undef, uri=undef, bandwidth=0) -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [perl PATCH 0/6] Misc fixes to perl APIs and new migrate constant
Daniel P. Berrangé (6): Fix typo breaking some migrate parameter handling Add handling for VIR_MIGRATE_PARAM_PARALLEL_CONNECTIONS Add support for VIR_MIGRATE_PARAM_TLS_DESTINATION constant Add missing create_checkpoint method Remove docs for has_metadata method which doesn't exist Fix data type for VIR_CONNECT_IDENTITY_SASL_USER_NAME Changes | 9 + lib/Sys/Virt.xs | 25 + lib/Sys/Virt/Domain.pm | 26 ++ lib/Sys/Virt/DomainCheckpoint.pm | 5 - 4 files changed, 56 insertions(+), 9 deletions(-) -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [perl PATCH 4/6] Add missing create_checkpoint method
Signed-off-by: Daniel P. Berrangé
---
Changes| 1 +
lib/Sys/Virt/Domain.pm | 17 +
2 files changed, 18 insertions(+)
diff --git a/Changes b/Changes
index 09d1dca..f36d401 100644
--- a/Changes
+++ b/Changes
@@ -9,6 +9,7 @@ Revision history for perl module Sys::Virt
- Add handling for VIR_MIGRATE_PARAM_PARALLEL_CONNECTIONS
constant when invoking migrate
- Add VIR_MIGRATE_PARAM_TLS_DESTINATION constant
+ - Add missing create_checkpoint method on Sys::Virt::Domain
5.10.0 2019-12-03
diff --git a/lib/Sys/Virt/Domain.pm b/lib/Sys/Virt/Domain.pm
index ae909de..d9d64e4 100644
--- a/lib/Sys/Virt/Domain.pm
+++ b/lib/Sys/Virt/Domain.pm
@@ -1965,6 +1965,23 @@ sub get_checkpoint_by_name {
return Sys::Virt::DomainCheckpoint->_new(domain => $self, name => $name);
}
+=item $checkpoint = $dom->create_checkpoint($xml[, $flags])
+
+Create a new checkpoint from the C<$xml>. The C<$flags> parameter accepts
+the B constants listed in C.
+
+=cut
+
+sub create_checkpoint {
+my $self = shift;
+my $xml = shift;
+my $flags = shift;
+
+my $checkpoint = Sys::Virt::DomainCheckpoint->_new(domain => $self, xml =>
$xml, flags => $flags);
+
+return $checkpoint;
+}
+
1;
=item $dom->fs_trim($mountPoint, $minimum, $flags=0);
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [perl PATCH 6/6] Fix data type for VIR_CONNECT_IDENTITY_SASL_USER_NAME
Signed-off-by: Daniel P. Berrangé --- Changes | 2 ++ lib/Sys/Virt.xs | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Changes b/Changes index 9d9a5c8..e90e736 100644 --- a/Changes +++ b/Changes @@ -11,6 +11,8 @@ Revision history for perl module Sys::Virt - Add VIR_MIGRATE_PARAM_TLS_DESTINATION constant - Add missing create_checkpoint method on Sys::Virt::Domain - Remove docs for has_metadata method which doesn't exit + - Fix data type for VIR_CONNECT_IDENTITY_SASL_USER_NAME + parameter 5.10.0 2019-12-03 diff --git a/lib/Sys/Virt.xs b/lib/Sys/Virt.xs index 3245adf..ab12ef0 100644 --- a/lib/Sys/Virt.xs +++ b/lib/Sys/Virt.xs @@ -2738,7 +2738,7 @@ set_identity(conn, newident, flags=0) strncpy(ident[6].field, VIR_CONNECT_IDENTITY_SASL_USER_NAME, VIR_TYPED_PARAM_FIELD_LENGTH); - ident[6].type = VIR_TYPED_PARAM_INT; + ident[6].type = VIR_TYPED_PARAM_STRING; strncpy(ident[7].field, VIR_CONNECT_IDENTITY_X509_DISTINGUISHED_NAME, VIR_TYPED_PARAM_FIELD_LENGTH); -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [perl][PATCH] Add VIR_MIGRATE_PARAM_TLS_DESTINATION constant
On Tue, Dec 10, 2019 at 02:09:35PM +0100, Michal Privoznik wrote: > Signed-off-by: Michal Privoznik > --- > Changes| 1 + > lib/Sys/Virt.xs| 1 + > lib/Sys/Virt/Domain.pm | 9 + > 3 files changed, 11 insertions(+) > > diff --git a/Changes b/Changes > index 37529ac..cf5d8cc 100644 > --- a/Changes > +++ b/Changes > @@ -4,6 +4,7 @@ Revision history for perl module Sys::Virt > > - Add VIR_DOMAIN_JOB_SUCCESS and VIR_DOMAIN_JOB_STATS_KEEP_COMPLETED > constants > + - Add VIR_MIGRATE_PARAM_TLS_DESTINATION constant > > 5.10.0 2019-12-03 > > diff --git a/lib/Sys/Virt.xs b/lib/Sys/Virt.xs > index 98a12d7..a729a52 100644 > --- a/lib/Sys/Virt.xs > +++ b/lib/Sys/Virt.xs > @@ -9246,6 +9246,7 @@ BOOT: >REGISTER_CONSTANT_STR(VIR_MIGRATE_PARAM_AUTO_CONVERGE_INCREMENT, > MIGRATE_PARAM_AUTO_CONVERGE_INCREMENT); >REGISTER_CONSTANT_STR(VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY, > MIGRATE_PARAM_BANDWIDTH_POSTCOPY); >REGISTER_CONSTANT_STR(VIR_MIGRATE_PARAM_PARALLEL_CONNECTIONS, > MIGRATE_PARAM_PARALLEL_CONNECTIONS); > + REGISTER_CONSTANT_STR(VIR_MIGRATE_PARAM_TLS_DESTINATION, > MIGRATE_PARAM_TLS_DESTINATION); > >REGISTER_CONSTANT(VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY, > MIGRATE_MAX_SPEED_POSTCOPY); This is incomplete, as the migrate methods need to be updated to set the data type. Don't bother about a v2 though as I'm co-incidentally fixing this and many other perl bugs right now. > > diff --git a/lib/Sys/Virt/Domain.pm b/lib/Sys/Virt/Domain.pm > index 2caec51..ae909de 100644 > --- a/lib/Sys/Virt/Domain.pm > +++ b/lib/Sys/Virt/Domain.pm > @@ -1141,6 +1141,15 @@ progress is not made > > The number of connections used during parallel migration. > > +=item C > + > +Override the destination host name used for TLS verification. > +Normally the TLS certificate from the destination host must match > +the host's name for TLS verification to succeed. When the > +certificate does not match the destination hostname and the > +expected cetificate's hostname is known, this parameter can be > +used to pass this expected hostname when starting the migration. I'll borrow your description for my patch though 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 2/2] Add -mem-shared option
On Tue, 10 Dec 2019 11:34:32 +0100 Markus Armbruster wrote: > Eduardo Habkost writes: > > > +Markus > > > > On Tue, Dec 03, 2019 at 03:43:03PM +0100, Igor Mammedov wrote: > >> On Tue, 3 Dec 2019 09:56:15 +0100 > >> Thomas Huth wrote: > >> > >> > On 02/12/2019 22.00, Eduardo Habkost wrote: > >> > > On Mon, Dec 02, 2019 at 08:39:48AM +0100, Igor Mammedov wrote: > >> > >> On Fri, 29 Nov 2019 18:46:12 +0100 > >> > >> Paolo Bonzini wrote: > >> > >> > >> > >>> On 29/11/19 13:16, Igor Mammedov wrote: > >> > As for "-m", I'd make it just an alias that translates > >> > -m/mem-path/mem-prealloc > >> > >>> > >> > >>> I think we should just deprecate -mem-path/-mem-prealloc in 5.0. > >> > >>> CCing > >> > >>> Thomas as mister deprecation. :) > >> > >> > >> > >> I'll add that to my series > >> > > > >> > > Considering that the plan is to eventually reimplement those > >> > > options as syntactic sugar for memory backend options (hopefully > >> > > in less than 2 QEMU releases), what's the point of deprecating > >> > > them? > >> > > >> > Well, it depends on the "classification" [1] of the parameter... > >> > > >> > Let's ask: What's the main purpose of the option? > >> > > >> > Is it easier to use than the "full" option, and thus likely to be used > >> > by a lot of people who run QEMU directly from the CLI? In that case it > >> > should stay as "convenience option" and not be deprecated. > >> > > >> > Or is the option merely there to give the upper layers like libvirt or > >> > some few users and their scripts some more grace period to adapt their > >> > code, but we all agree that the options are rather ugly and should > >> > finally go away? Then it's rather a "legacy option" and the deprecation > >> > process is the right way to go. Our QEMU interface is still way > >> > overcrowded, we should try to keep it as clean as possible. > >> > >> After switching to memdev for main RAM, users could use relatively > >> short global options > >> -global memory-backend.prealloc|share=on > >> and > >> -global memory-backend-file.mem-path=X|prealloc|share=on > >> > >> instead of us adding and maintaining slightly shorter > >> -mem-shared/-mem-path/-mem-prealloc > > > > Global properties are a convenient way to expose knobs through > > the command line with little effort, but we have no documentation > > on which QOM properties are really supposed to be touched by > > users using -global. > > > > Unless we fix the lack of documentation, I'd prefer to have > > syntactic sugar translated to -global instead of recommending > > direct usage of -global. > > Fair point. > > I'd take QOM property documentation over still more sugar. > > Sometimes, the practical way to make simple things simple is sugar. I > can accept that. This doesn't look like such a case, though. I can document concrete globals as replacement at the place -mem-path/-mem-prealloc are documented during deprecation and then in 2 releases we will just drop legacy syntax and keep only globals over there. (eventually it will spread various globals over man page, which I don't like but we probably should start somwhere and consolidate later if globals in man page become normal practice.) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [perl][PATCH] Add VIR_MIGRATE_PARAM_TLS_DESTINATION constant
Signed-off-by: Michal Privoznik --- Changes| 1 + lib/Sys/Virt.xs| 1 + lib/Sys/Virt/Domain.pm | 9 + 3 files changed, 11 insertions(+) diff --git a/Changes b/Changes index 37529ac..cf5d8cc 100644 --- a/Changes +++ b/Changes @@ -4,6 +4,7 @@ Revision history for perl module Sys::Virt - Add VIR_DOMAIN_JOB_SUCCESS and VIR_DOMAIN_JOB_STATS_KEEP_COMPLETED constants + - Add VIR_MIGRATE_PARAM_TLS_DESTINATION constant 5.10.0 2019-12-03 diff --git a/lib/Sys/Virt.xs b/lib/Sys/Virt.xs index 98a12d7..a729a52 100644 --- a/lib/Sys/Virt.xs +++ b/lib/Sys/Virt.xs @@ -9246,6 +9246,7 @@ BOOT: REGISTER_CONSTANT_STR(VIR_MIGRATE_PARAM_AUTO_CONVERGE_INCREMENT, MIGRATE_PARAM_AUTO_CONVERGE_INCREMENT); REGISTER_CONSTANT_STR(VIR_MIGRATE_PARAM_BANDWIDTH_POSTCOPY, MIGRATE_PARAM_BANDWIDTH_POSTCOPY); REGISTER_CONSTANT_STR(VIR_MIGRATE_PARAM_PARALLEL_CONNECTIONS, MIGRATE_PARAM_PARALLEL_CONNECTIONS); + REGISTER_CONSTANT_STR(VIR_MIGRATE_PARAM_TLS_DESTINATION, MIGRATE_PARAM_TLS_DESTINATION); REGISTER_CONSTANT(VIR_DOMAIN_MIGRATE_MAX_SPEED_POSTCOPY, MIGRATE_MAX_SPEED_POSTCOPY); diff --git a/lib/Sys/Virt/Domain.pm b/lib/Sys/Virt/Domain.pm index 2caec51..ae909de 100644 --- a/lib/Sys/Virt/Domain.pm +++ b/lib/Sys/Virt/Domain.pm @@ -1141,6 +1141,15 @@ progress is not made The number of connections used during parallel migration. +=item C + +Override the destination host name used for TLS verification. +Normally the TLS certificate from the destination host must match +the host's name for TLS verification to succeed. When the +certificate does not match the destination hostname and the +expected cetificate's hostname is known, this parameter can be +used to pass this expected hostname when starting the migration. + =back =item $ddom = $dom->migrate(destcon, flags=0, dname=undef, uri=undef, bandwidth=0) -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 00/14] drop usage of c-type gnulib module
On Sun, Dec 08, 2019 at 06:36:22PM -0500, Cole Robinson wrote: > On 11/20/19 9:48 AM, Pavel Hrdina wrote: > > Changes in v2: > > - fixed missing parentheses in patch 01 > > - added patch 02 to cover c_isascii > > > > With the two issues I pointed out fixed: > > Reviewed-by: Cole Robinson Thanks, fixed and pushed. Pavel signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: honour parseOpaque instead of refetching caps
The use of the parseOpaque parameter was mistakenly removed in
commit 4a4132b4625778cf80acb9c92d06351b44468ac3
Author: Daniel P. Berrangé
Date: Tue Dec 3 10:49:49 2019 +
conf: don't use passed in caps in post parse method
causing the method to re-fetch qemuCaps that were already just
fetched and put into parseOpaque.
This is inefficient when parsing incoming XML, but for live
XML this is more serious as it means we use the capabilities
for the current QEMU binary on disk, rather than the running
QEMU.
That commit, however, did have a useful side effect of fixing
a crasher bug in the qemu post parse callback introduced by
commit 5e939cea896fb3373a6f68f86e325c657429ed3d
Author: Jiri Denemark
Date: Thu Sep 26 18:42:02 2019 +0200
qemu: Store default CPU in domain XML
The qemuDomainDefSetDefaultCPU() method in that patch did not
allow for the possibility that qemuCaps would be NULL and thus
resulted in a SEGV.
This shows a risk in letting each check in the post parse
callback look for qemuCaps == NULL. The safer option is to
check once upfront and immediately stop (postpone) further
validation.
Honouring the cached caps for the live status XML, highlights
a second flaw, which is that it shouldn't check the virt
type or arch for running guests. The info needed to check this
is not in the cache caps, only the list of flags are populated.
Thus some of the post parse logic is made to only run for
inactive guests. This showed a bug in one test data file which
lacked an ID attribute for the live guest.
Signed-off-by: Daniel P. Berrangé
---
src/qemu/qemu_domain.c| 52 +++
.../qemustatusxml2xmldata/vcpus-multi-in.xml | 2 +-
2 files changed, 31 insertions(+), 23 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 6f53e17b6a..2abe93c829 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4703,16 +4703,17 @@ static int
qemuDomainDefPostParse(virDomainDefPtr def,
unsigned int parseFlags,
void *opaque,
- void *parseOpaque G_GNUC_UNUSED)
+ void *parseOpaque)
{
virQEMUDriverPtr driver = opaque;
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
-g_autoptr(virQEMUCaps) qemuCaps = NULL;
+/* Note that qemuCaps may be NULL when this function is called. This
+ * function shall not fail in that case. It will be re-run on VM startup
+ * with the capabilities populated. */
+virQEMUCapsPtr qemuCaps = parseOpaque;
-if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache,
-def->emulator))) {
+if (!qemuCaps)
return 1;
-}
if (def->os.type != VIR_DOMAIN_OSTYPE_HVM) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -4721,18 +4722,31 @@ qemuDomainDefPostParse(virDomainDefPtr def,
return -1;
}
-if (!virQEMUCapsIsArchSupported(qemuCaps, def->os.arch)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Emulator '%s' does not support arch '%s'"),
- def->emulator, virArchToString(def->os.arch));
-return -1;
-}
+if (def->id == -1) {
+if (!virQEMUCapsIsArchSupported(qemuCaps, def->os.arch)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Emulator '%s' does not support arch '%s'"),
+ def->emulator, virArchToString(def->os.arch));
+return -1;
+}
-if (!virQEMUCapsIsVirtTypeSupported(qemuCaps, def->virtType)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Emulator '%s' does not support virt type '%s'"),
- def->emulator,
virDomainVirtTypeToString(def->virtType));
-return -1;
+if (!virQEMUCapsIsVirtTypeSupported(qemuCaps, def->virtType)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Emulator '%s' does not support virt type '%s'"),
+ def->emulator,
virDomainVirtTypeToString(def->virtType));
+return -1;
+}
+if (!def->os.machine) {
+const char *machine = virQEMUCapsGetPreferredMachine(qemuCaps,
+
def->virtType);
+def->os.machine = g_strdup(machine);
+}
+} else {
+if (!def->os.machine) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("missing machine type"));
+return -1;
+}
}
if (def->os.bootloader || def->os.bootloaderArgs) {
@@ -4741,12 +4755,6 @@ qemuDomainDefPostParse(virDomainDefPtr def,
return -1;
}
-if (!def->os.machine) {
-const char *machine = virQEMUCapsGetPreferredMachine(qemuCaps,
-
Re: [libvirt] libvirt-guests.sh without or with failing ACPI support
On Tue, Dec 10, 2019 at 12:41 PM Henning Schild wrote: > Am Tue, 10 Dec 2019 12:01:24 +0100 > schrieb Christian Ehrhardt : > > > On Tue, Dec 10, 2019 at 10:46 AM Henning Schild > > wrote: > > > > > Hi all, > > > > > > the systemd shutdown scripts work sequentially with a 300s timeout > > > (seen on Debian). If a VM does not have ACPI support, or the ACPI > > > support failed for some reason, you are looking at a 300s timeout > > > per instance for a host shutdown/reboot. > > > i.e. 10 instances without working ACPI = 3000s to shut down > > > > > > I think the systemd scripting should be parallel instead of > > > sequentially. So if you have many VMs without working ACPI you just > > > have to wait 300s in total for the host to shut down. > > > > > > > Hi Henning, > > this is configurable in /etc/default/libvirt-guests > > For example Ubuntu (otherwise using the same bits) changes that to run > > PARALLEL_SHUTDOWN=10 > > SHUTDOWN_TIMEOUT=120 > > Sweet. I went for the PARALLEL_SHUTDOWN=10 and left the 300. Maybe the > default PARALLEL_SHUTDOWN value should not be 0 ? > > > I never got bugs about that config being too aggressive. > > The change is old and as easy as: > > > https://git.launchpad.net/ubuntu/+source/libvirt/tree/debian/patches/ubuntu/parallel-shutdown.patch?h=ubuntu/focal-devel > > Maybe you just want to open a bug with Debian to change the default > > config there as well? > > No it is a bug in libvirt having the "wrong" defaults. And a bug in > ubuntu not fixing it upstream ;). > No it isn't as it was discussed and nacked (years ago) > Thanks, > Henning > > > Steps to reproduce: > > > - star a VM that does not support ACPI > > > - reboot the host and wait 300s for the VM to be shut down > > > - now start it multiple times > > > - wait multiples of 300s for the shutdown > > > > > > Expected behaviour: > > > - no matter how many instances do not support ACPI, make it 300s > > > max because we shut them down in parallel > > > > > > > > > regards, > > > Henning > > > > > > > > > -- > > > libvir-list mailing list > > > libvir-list@redhat.com > > > https://www.redhat.com/mailman/listinfo/libvir-list > > > > > > > > > > -- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH 0/2] Build Go projects on CentOS 7 once again
On Tuesday, December 10, 2019, Andrea Bolognani wrote: > Andrea Bolognani (2): > guests: Enable Go projects on CentOS 7 once again > Build Go projects on CentOS 7 once again > > guests/host_vars/libvirt-centos-7/main.yml | 2 ++ > guests/playbooks/build/projects/libvirt-go-xml.yml | 13 + > guests/playbooks/build/projects/libvirt-go.yml | 13 + > jenkins/projects/libvirt-go-xml.yaml | 9 + > jenkins/projects/libvirt-go.yaml | 9 + > 5 files changed, 6 insertions(+), 40 deletions(-) > > -- > 2.23.0 > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list > > Reviewed-by: Fabiano Fidêncio -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH 2/2] Build Go projects on CentOS 7 once again
Now that we have a recent Go compiler installed, we can once again
start building all projects written in that language on the target.
Signed-off-by: Andrea Bolognani
---
guests/playbooks/build/projects/libvirt-go-xml.yml | 13 +
guests/playbooks/build/projects/libvirt-go.yml | 13 +
jenkins/projects/libvirt-go-xml.yaml | 9 +
jenkins/projects/libvirt-go.yaml | 9 +
4 files changed, 4 insertions(+), 40 deletions(-)
diff --git a/guests/playbooks/build/projects/libvirt-go-xml.yml
b/guests/playbooks/build/projects/libvirt-go-xml.yml
index 5cacb7c..ef932b4 100644
--- a/guests/playbooks/build/projects/libvirt-go-xml.yml
+++ b/guests/playbooks/build/projects/libvirt-go-xml.yml
@@ -1,18 +1,7 @@
---
- set_fact:
name: libvirt-go-xml
-machines:
- - libvirt-debian-9
- - libvirt-debian-10
- - libvirt-debian-sid
- - libvirt-fedora-30
- - libvirt-fedora-31
- - libvirt-fedora-rawhide
- - libvirt-freebsd-11
- - libvirt-freebsd-12
- - libvirt-freebsd-current
- - libvirt-ubuntu-1604
- - libvirt-ubuntu-1804
+machines: '{{ all_machines }}'
archive_format: gz
git_url: '{{ git_urls["libvirt-go-xml"][git_remote] }}'
diff --git a/guests/playbooks/build/projects/libvirt-go.yml
b/guests/playbooks/build/projects/libvirt-go.yml
index 36322b1..6c59900 100644
--- a/guests/playbooks/build/projects/libvirt-go.yml
+++ b/guests/playbooks/build/projects/libvirt-go.yml
@@ -1,18 +1,7 @@
---
- set_fact:
name: libvirt-go
-machines:
- - libvirt-debian-9
- - libvirt-debian-10
- - libvirt-debian-sid
- - libvirt-fedora-30
- - libvirt-fedora-31
- - libvirt-fedora-rawhide
- - libvirt-freebsd-11
- - libvirt-freebsd-12
- - libvirt-freebsd-current
- - libvirt-ubuntu-1604
- - libvirt-ubuntu-1804
+machines: '{{ all_machines }}'
archive_format: gz
git_url: '{{ git_urls["libvirt-go"][git_remote] }}'
diff --git a/jenkins/projects/libvirt-go-xml.yaml
b/jenkins/projects/libvirt-go-xml.yaml
index 8b4e2c5..73746fd 100644
--- a/jenkins/projects/libvirt-go-xml.yaml
+++ b/jenkins/projects/libvirt-go-xml.yaml
@@ -1,14 +1,7 @@
---
- project:
name: libvirt-go-xml
-machines:
- - libvirt-debian-9
- - libvirt-debian-10
- - libvirt-fedora-30
- - libvirt-fedora-31
- - libvirt-fedora-rawhide
- - libvirt-freebsd-11
- - libvirt-freebsd-12
+machines: '{ all_machines }'
title: Libvirt Go XML
archive_format: gz
git_url: '{git_urls[libvirt-go-xml][default]}'
diff --git a/jenkins/projects/libvirt-go.yaml b/jenkins/projects/libvirt-go.yaml
index 63d039a..4ab9fd2 100644
--- a/jenkins/projects/libvirt-go.yaml
+++ b/jenkins/projects/libvirt-go.yaml
@@ -1,14 +1,7 @@
---
- project:
name: libvirt-go
-machines:
- - libvirt-debian-9
- - libvirt-debian-10
- - libvirt-fedora-30
- - libvirt-fedora-31
- - libvirt-fedora-rawhide
- - libvirt-freebsd-11
- - libvirt-freebsd-12
+machines: '{ all_machines }'
title: Libvirt Go
archive_format: gz
git_url: '{git_urls[libvirt-go][default]}'
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH 1/2] guests: Enable Go projects on CentOS 7 once again
We had disabled this a year ago with 72afaed959f8; however, a few months ago we started enabling EPEL on CentOS 7 with d63d6a59cc11, and from that source we can install a recent Go compiler. Signed-off-by: Andrea Bolognani --- guests/host_vars/libvirt-centos-7/main.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/guests/host_vars/libvirt-centos-7/main.yml b/guests/host_vars/libvirt-centos-7/main.yml index 94e29af..e0579ad 100644 --- a/guests/host_vars/libvirt-centos-7/main.yml +++ b/guests/host_vars/libvirt-centos-7/main.yml @@ -5,6 +5,8 @@ projects: - libvirt-cim - libvirt-dbus - libvirt-glib + - libvirt-go + - libvirt-go-xml - libvirt-ocaml - libvirt-perl - libvirt-python -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH 0/2] Build Go projects on CentOS 7 once again
Andrea Bolognani (2): guests: Enable Go projects on CentOS 7 once again Build Go projects on CentOS 7 once again guests/host_vars/libvirt-centos-7/main.yml | 2 ++ guests/playbooks/build/projects/libvirt-go-xml.yml | 13 + guests/playbooks/build/projects/libvirt-go.yml | 13 + jenkins/projects/libvirt-go-xml.yaml | 9 + jenkins/projects/libvirt-go.yaml | 9 + 5 files changed, 6 insertions(+), 40 deletions(-) -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/30] conf: move virt type / os type / arch validation to post-parse
On Tue, Dec 10, 2019 at 10:57:44AM +, Daniel P. Berrangé wrote:
> On Mon, Dec 09, 2019 at 01:21:05PM -0500, Cole Robinson wrote:
> > On 12/4/19 9:20 AM, Daniel P. Berrangé wrote:
> > > The XML parser currently calls virCapabilitiesDomainDataLookup during
> > > parsing to find the domain capabilities matching the triple
> > >
> > > (virt type, os type, arch)
> > >
> > > This is, however, bogus with the QEMU driver as it assumes that there
> > > is an emulator known to the default driver capabilities that matches
> > > this triple. It is entirely possible for the driver to be parsing an
> > > XML file with a custom emulator path specified pointing to a binary
> > > that doesn't exist in the default driver capabilities. This will,
> > > for example be the case on a RHEL host which only installs the host
> > > native emulator to /usr/bin. The user can have built a custom QEMU
> > > for non-native arches into $HOME and wish to use that.
> > >
> > > Aside from validation, this call is also used to fill in a machine type
> > > for the guest if not otherwise specified. Again, this data may be
> > > incorrect for the QEMU driver because it is not taking account of
> > > the emulator binary that is referenced.
> > >
> > > To start fixing this, move the validation to the post-parse callbacks
> > > where more intelligent driver specific logic can be applied.
> > >
> > > Signed-off-by: Daniel P. Berrangé
> > > ---
> > > src/bhyve/bhyve_domain.c | 7 ++-
> > > src/conf/capabilities.c| 17 +
> > > src/conf/capabilities.h| 7 +++
> > > src/conf/domain_conf.c | 19 ++-
> > > src/libvirt_private.syms | 1 +
> > > src/libxl/libxl_domain.c | 7 ++-
> > > src/lxc/lxc_domain.c | 5 +
> > > src/openvz/openvz_conf.c | 7 ++-
> > > src/phyp/phyp_driver.c | 9 +++--
> > > src/qemu/qemu_domain.c | 19 +++
> > > src/vmware/vmware_driver.c | 9 +++--
> > > src/vmx/vmx.c | 9 +++--
> > > src/vz/vz_driver.c | 7 ++-
> > > 13 files changed, 92 insertions(+), 31 deletions(-)
> > >
> > > diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c
> > > index 7d24bb602f..575f141b53 100644
> > > --- a/src/bhyve/bhyve_domain.c
> > > +++ b/src/bhyve/bhyve_domain.c
> > > @@ -74,11 +74,16 @@ bhyveDomainDefNeedsISAController(virDomainDefPtr def)
> > >
> > > static int
> > > bhyveDomainDefPostParse(virDomainDefPtr def,
> > > -virCapsPtr caps G_GNUC_UNUSED,
> > > +virCapsPtr caps,
> > > unsigned int parseFlags G_GNUC_UNUSED,
> > > void *opaque G_GNUC_UNUSED,
> > > void *parseOpaque G_GNUC_UNUSED)
> > > {
> > > +if (!virCapabilitiesDomainSupported(caps, def->os.type,
> > > +def->os.arch,
> > > +def->virtType))
> > > +return -1;
> > > +
> > > /* Add an implicit PCI root controller */
> > > if (virDomainDefMaybeAddController(def,
> > > VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0,
> > >
> > > VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) < 0)
> > > diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> > > index ff7d265621..748dd64273 100644
> > > --- a/src/conf/capabilities.c
> > > +++ b/src/conf/capabilities.c
> > > @@ -816,6 +816,23 @@ virCapabilitiesDomainDataLookup(virCapsPtr caps,
> > > }
> > >
> > >
> > > +bool
> > > +virCapabilitiesDomainSupported(virCapsPtr caps,
> > > + int ostype,
> > > + virArch arch,
> > > + int virttype)
> > > +{
> > > +g_autofree virCapsDomainDataPtr capsdata = NULL;
> > > +
> > > +capsdata = virCapabilitiesDomainDataLookup(caps, ostype,
> > > + arch,
> > > + virttype,
> > > + NULL, NULL);
> > > +
> > > +return capsdata != NULL;
> > > +}
> > > +
> > > +
> > > int
> > > virCapabilitiesAddStoragePool(virCapsPtr caps,
> > >int poolType)
> > > diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
> > > index 8a7137d7eb..c39fe0de08 100644
> > > --- a/src/conf/capabilities.h
> > > +++ b/src/conf/capabilities.h
> > > @@ -309,6 +309,13 @@ virCapabilitiesDomainDataLookup(virCapsPtr caps,
> > > const char *emulator,
> > > const char *machinetype);
> > >
> > > +bool
> > > +virCapabilitiesDomainSupported(virCapsPtr caps,
> > > + int ostype,
> > > + virArch arch,
> > > + int domaintype);
> > > +
> > > +
> > > void
> > >
Re: [libvirt] libvirt-guests.sh without or with failing ACPI support
Am Tue, 10 Dec 2019 12:01:24 +0100 schrieb Christian Ehrhardt : > On Tue, Dec 10, 2019 at 10:46 AM Henning Schild > wrote: > > > Hi all, > > > > the systemd shutdown scripts work sequentially with a 300s timeout > > (seen on Debian). If a VM does not have ACPI support, or the ACPI > > support failed for some reason, you are looking at a 300s timeout > > per instance for a host shutdown/reboot. > > i.e. 10 instances without working ACPI = 3000s to shut down > > > > I think the systemd scripting should be parallel instead of > > sequentially. So if you have many VMs without working ACPI you just > > have to wait 300s in total for the host to shut down. > > > > Hi Henning, > this is configurable in /etc/default/libvirt-guests > For example Ubuntu (otherwise using the same bits) changes that to run > PARALLEL_SHUTDOWN=10 > SHUTDOWN_TIMEOUT=120 Sweet. I went for the PARALLEL_SHUTDOWN=10 and left the 300. Maybe the default PARALLEL_SHUTDOWN value should not be 0 ? > I never got bugs about that config being too aggressive. > The change is old and as easy as: > https://git.launchpad.net/ubuntu/+source/libvirt/tree/debian/patches/ubuntu/parallel-shutdown.patch?h=ubuntu/focal-devel > Maybe you just want to open a bug with Debian to change the default > config there as well? No it is a bug in libvirt having the "wrong" defaults. And a bug in ubuntu not fixing it upstream ;). Thanks, Henning > Steps to reproduce: > > - star a VM that does not support ACPI > > - reboot the host and wait 300s for the VM to be shut down > > - now start it multiple times > > - wait multiples of 300s for the shutdown > > > > Expected behaviour: > > - no matter how many instances do not support ACPI, make it 300s > > max because we shut them down in parallel > > > > > > regards, > > Henning > > > > > > -- > > 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 1/2] net: Drop the legacy "name" parameter from the -net option
On 10/12/2019 12.06, Markus Armbruster wrote: > Thomas Huth writes: > >> It's been deprecated since QEMU v3.1, so it's time to finally >> remove it. The "id" parameter can simply be used instead. >> >> Signed-off-by: Thomas Huth > [...] >> diff --git a/qapi/net.json b/qapi/net.json >> index 335295be50..ff280ccd16 100644 >> --- a/qapi/net.json >> +++ b/qapi/net.json >> @@ -488,18 +488,16 @@ >> # >> # @id: identifier for monitor commands >> # >> -# @name: identifier for monitor commands, ignored if @id is present >> -# >> # @opts: device type specific properties (legacy) >> # >> # Since: 1.2 >> # >> # 'vlan': dropped in 3.0 >> +# 'name': dropped in 5.0 >> ## > > Uh, when did we start to add "dropped in" to our doc comments? > > We should either do this systematically, or not at all. If the former, > we have quite a few "dropped in" to add belatedly. IIRC this has been suggested by Eric, see e.g.: https://patchwork.kernel.org/patch/10227335/#21501161 Anyway, it should not matter much for this patch here, since the line gets removed again in the 2nd patch anyway. Thomas -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] net: Drop the NetLegacy structure, always use Netdev instead
Thomas Huth writes:
> Now that the "name" parameter is gone, there is hardly any difference
> between NetLegacy and Netdev anymore. Drop NetLegacy and always use
> Netdev to simplify the code quite a bit.
>
> Signed-off-by: Thomas Huth
Took me a minute to see the actual difference.
Here's Netdev:
{ 'union': 'Netdev',
'base': { 'id': 'str', 'type': 'NetClientDriver' },
'discriminator': 'type',
'data': {
'nic': 'NetLegacyNicOptions',
'user': 'NetdevUserOptions',
'tap': 'NetdevTapOptions',
'l2tpv3': 'NetdevL2TPv3Options',
'socket': 'NetdevSocketOptions',
'vde': 'NetdevVdeOptions',
'bridge': 'NetdevBridgeOptions',
'hubport': 'NetdevHubPortOptions',
'netmap': 'NetdevNetmapOptions',
'vhost-user': 'NetdevVhostUserOptions' } }
{ 'enum': 'NetClientDriver',
'data': [ 'none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde',
'bridge', 'hubport', 'netmap', 'vhost-user' ] }
Here's NetLegacy:
{ 'struct': 'NetLegacy',
'data': {
'*id': 'str',
'opts': 'NetLegacyOptions' } }
{ 'union': 'NetLegacyOptions',
'base': { 'type': 'NetLegacyOptionsType' },
'discriminator': 'type',
'data': {
'nic': 'NetLegacyNicOptions',
'user': 'NetdevUserOptions',
'tap': 'NetdevTapOptions',
'l2tpv3': 'NetdevL2TPv3Options',
'socket': 'NetdevSocketOptions',
'vde': 'NetdevVdeOptions',
'bridge': 'NetdevBridgeOptions',
'netmap': 'NetdevNetmapOptions',
'vhost-user': 'NetdevVhostUserOptions' } }
{ 'enum': 'NetLegacyOptionsType',
'data': ['none', 'nic', 'user', 'tap', 'l2tpv3', 'socket', 'vde',
'bridge', 'netmap', 'vhost-user'] }
Difference: NetLegacy wraps the union in @opts.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] net: Drop the legacy "name" parameter from the -net option
Thomas Huth writes:
> It's been deprecated since QEMU v3.1, so it's time to finally
> remove it. The "id" parameter can simply be used instead.
>
> Signed-off-by: Thomas Huth
[...]
> diff --git a/qapi/net.json b/qapi/net.json
> index 335295be50..ff280ccd16 100644
> --- a/qapi/net.json
> +++ b/qapi/net.json
> @@ -488,18 +488,16 @@
> #
> # @id: identifier for monitor commands
> #
> -# @name: identifier for monitor commands, ignored if @id is present
> -#
> # @opts: device type specific properties (legacy)
> #
> # Since: 1.2
> #
> # 'vlan': dropped in 3.0
> +# 'name': dropped in 5.0
> ##
Uh, when did we start to add "dropped in" to our doc comments?
We should either do this systematically, or not at all. If the former,
we have quite a few "dropped in" to add belatedly.
I vote for "not at all".
> { 'struct': 'NetLegacy',
>'data': {
> '*id': 'str',
> -'*name': 'str',
> 'opts': 'NetLegacyOptions' } }
>
> ##
[...]
History:
$ git-log -S"dropped in" -- qapi qapi-schema.json
commit ffaee83bcb28913b8b854aeab78b1a1f2115712d
Author: Markus Armbruster
Date: Tue Jul 9 17:20:53 2019 +0200
qapi: Move query-target from misc.json to machine.json
Move query-target and its return type TargetInfo from misc.json to
machine.json, where they are covered by MAINTAINERS section "Machine
core". Also move its implementation from arch_init.c to
hw/core/machine-qmp-cmds, where it is likewise covered.
All users of SysEmuTarget are now in machine.json. Move it there from
common.json.
Signed-off-by: Markus Armbruster
Message-Id: <20190709152053.16670-3-arm...@redhat.com>
commit 416756cc049006ab8a05fe39e5f2e6af25cad9d2
Author: Thomas Huth
Date: Tue Aug 21 13:27:48 2018 +0200
Record history of ppcemb target in common.json
We recently removed the long deprecated "ppcemb" target. This adds a
comment in common.json about the SysEmuTarget type, recording when it was
removed.
Suggested-by: Eric Blake
Signed-off-by: David Gibson
commit 4088b5536436090207dcf6d15e47908f74b2d8f2
Author: Thomas Huth
Date: Tue May 15 18:26:20 2018 +0200
qapi/net.json: Fix the version number of the "vlan" removal
"vlan" will be dropped in 2.13, not in 2.12. And while we're at it,
use the better wording "dropped in" instead of "removed with" (also
for the "dump" removal).
Reported-by: Stefan Hajnoczi
Reported-by: Eric Blake
Signed-off-by: Thomas Huth
Reviewed-by: Eric Blake
Signed-off-by: Michael Tokarev
commit 608cfed66a6adeac136b0c09cd62d081062256f3
Author: Markus Armbruster
Date: Thu Aug 24 21:14:00 2017 +0200
qapi-schema: Collect UI stuff in qapi/ui.json
UI stuff is remote desktop stuff (Spice, VNC) and input stuff (mouse,
keyboard).
Cc: Gerd Hoffmann
Signed-off-by: Markus Armbruster
Message-Id: <1503602048-12268-9-git-send-email-arm...@redhat.com>
Reviewed-by: Marc-André Lureau
Reviewed-by: Gerd Hoffmann
commit 912092b8e47f31c3db25e088af8460d9e752da29
Author: Gerd Hoffmann
Date: Thu Jul 27 12:47:20 2017 +0200
ui: drop altgr and altgr_r QKeyCodes
The right alt key (alt_r aka KEY_RIGHTALT) is used for AltGr.
The altgr and altgr_r keys simply don't exist. Drop them.
Signed-off-by: Gerd Hoffmann
Reviewed-by: Eric Blake
Message-id: 20170727104720.30061-1-kra...@redhat.com
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [jenkins-ci PATCH] Sync comments
Some comments were only present in the Ansible part and not in
the Jenkins part and vice versa.
Signed-off-by: Andrea Bolognani
---
Pushed as trivial.
guests/playbooks/build/projects/libvirt-tck.yml | 2 ++
jenkins/projects/libosinfo.yaml | 2 --
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/guests/playbooks/build/projects/libvirt-tck.yml
b/guests/playbooks/build/projects/libvirt-tck.yml
index ef88cd3..b8d848c 100644
--- a/guests/playbooks/build/projects/libvirt-tck.yml
+++ b/guests/playbooks/build/projects/libvirt-tck.yml
@@ -1,5 +1,7 @@
---
- set_fact:
+# CentOS 7 doesn't include perl-generators, which is necessary to
+# build libvirt-tck
name: libvirt-tck
machines:
- libvirt-debian-9
diff --git a/jenkins/projects/libosinfo.yaml b/jenkins/projects/libosinfo.yaml
index cde10ec..819c5ac 100644
--- a/jenkins/projects/libosinfo.yaml
+++ b/jenkins/projects/libosinfo.yaml
@@ -1,8 +1,6 @@
---
- project:
name: libosinfo
-# libosinfo depends on meson 0.49.0, which is not available on
-# CentOS 7, Debian 9, Ubuntu 18;
machines: '{all_machines}'
title: libosinfo
archive_format: xz
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt-guests.sh without or with failing ACPI support
On Tue, Dec 10, 2019 at 10:46 AM Henning Schild wrote: > Hi all, > > the systemd shutdown scripts work sequentially with a 300s timeout > (seen on Debian). If a VM does not have ACPI support, or the ACPI > support failed for some reason, you are looking at a 300s timeout per > instance for a host shutdown/reboot. > i.e. 10 instances without working ACPI = 3000s to shut down > > I think the systemd scripting should be parallel instead of > sequentially. So if you have many VMs without working ACPI you just > have to wait 300s in total for the host to shut down. > Hi Henning, this is configurable in /etc/default/libvirt-guests For example Ubuntu (otherwise using the same bits) changes that to run PARALLEL_SHUTDOWN=10 SHUTDOWN_TIMEOUT=120 I never got bugs about that config being too aggressive. The change is old and as easy as: https://git.launchpad.net/ubuntu/+source/libvirt/tree/debian/patches/ubuntu/parallel-shutdown.patch?h=ubuntu/focal-devel Maybe you just want to open a bug with Debian to change the default config there as well? Steps to reproduce: > - star a VM that does not support ACPI > - reboot the host and wait 300s for the VM to be shut down > - now start it multiple times > - wait multiples of 300s for the shutdown > > Expected behaviour: > - no matter how many instances do not support ACPI, make it 300s max >because we shut them down in parallel > > > regards, > Henning > > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list > > -- Christian Ehrhardt Staff Engineer, Ubuntu Server Canonical Ltd -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/30] conf: move virt type / os type / arch validation to post-parse
On Mon, Dec 09, 2019 at 01:21:05PM -0500, Cole Robinson wrote:
> On 12/4/19 9:20 AM, Daniel P. Berrangé wrote:
> > The XML parser currently calls virCapabilitiesDomainDataLookup during
> > parsing to find the domain capabilities matching the triple
> >
> > (virt type, os type, arch)
> >
> > This is, however, bogus with the QEMU driver as it assumes that there
> > is an emulator known to the default driver capabilities that matches
> > this triple. It is entirely possible for the driver to be parsing an
> > XML file with a custom emulator path specified pointing to a binary
> > that doesn't exist in the default driver capabilities. This will,
> > for example be the case on a RHEL host which only installs the host
> > native emulator to /usr/bin. The user can have built a custom QEMU
> > for non-native arches into $HOME and wish to use that.
> >
> > Aside from validation, this call is also used to fill in a machine type
> > for the guest if not otherwise specified. Again, this data may be
> > incorrect for the QEMU driver because it is not taking account of
> > the emulator binary that is referenced.
> >
> > To start fixing this, move the validation to the post-parse callbacks
> > where more intelligent driver specific logic can be applied.
> >
> > Signed-off-by: Daniel P. Berrangé
> > ---
> > src/bhyve/bhyve_domain.c | 7 ++-
> > src/conf/capabilities.c| 17 +
> > src/conf/capabilities.h| 7 +++
> > src/conf/domain_conf.c | 19 ++-
> > src/libvirt_private.syms | 1 +
> > src/libxl/libxl_domain.c | 7 ++-
> > src/lxc/lxc_domain.c | 5 +
> > src/openvz/openvz_conf.c | 7 ++-
> > src/phyp/phyp_driver.c | 9 +++--
> > src/qemu/qemu_domain.c | 19 +++
> > src/vmware/vmware_driver.c | 9 +++--
> > src/vmx/vmx.c | 9 +++--
> > src/vz/vz_driver.c | 7 ++-
> > 13 files changed, 92 insertions(+), 31 deletions(-)
> >
> > diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c
> > index 7d24bb602f..575f141b53 100644
> > --- a/src/bhyve/bhyve_domain.c
> > +++ b/src/bhyve/bhyve_domain.c
> > @@ -74,11 +74,16 @@ bhyveDomainDefNeedsISAController(virDomainDefPtr def)
> >
> > static int
> > bhyveDomainDefPostParse(virDomainDefPtr def,
> > -virCapsPtr caps G_GNUC_UNUSED,
> > +virCapsPtr caps,
> > unsigned int parseFlags G_GNUC_UNUSED,
> > void *opaque G_GNUC_UNUSED,
> > void *parseOpaque G_GNUC_UNUSED)
> > {
> > +if (!virCapabilitiesDomainSupported(caps, def->os.type,
> > +def->os.arch,
> > +def->virtType))
> > +return -1;
> > +
> > /* Add an implicit PCI root controller */
> > if (virDomainDefMaybeAddController(def,
> > VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0,
> >
> > VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) < 0)
> > diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> > index ff7d265621..748dd64273 100644
> > --- a/src/conf/capabilities.c
> > +++ b/src/conf/capabilities.c
> > @@ -816,6 +816,23 @@ virCapabilitiesDomainDataLookup(virCapsPtr caps,
> > }
> >
> >
> > +bool
> > +virCapabilitiesDomainSupported(virCapsPtr caps,
> > + int ostype,
> > + virArch arch,
> > + int virttype)
> > +{
> > +g_autofree virCapsDomainDataPtr capsdata = NULL;
> > +
> > +capsdata = virCapabilitiesDomainDataLookup(caps, ostype,
> > + arch,
> > + virttype,
> > + NULL, NULL);
> > +
> > +return capsdata != NULL;
> > +}
> > +
> > +
> > int
> > virCapabilitiesAddStoragePool(virCapsPtr caps,
> >int poolType)
> > diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
> > index 8a7137d7eb..c39fe0de08 100644
> > --- a/src/conf/capabilities.h
> > +++ b/src/conf/capabilities.h
> > @@ -309,6 +309,13 @@ virCapabilitiesDomainDataLookup(virCapsPtr caps,
> > const char *emulator,
> > const char *machinetype);
> >
> > +bool
> > +virCapabilitiesDomainSupported(virCapsPtr caps,
> > + int ostype,
> > + virArch arch,
> > + int domaintype);
> > +
> > +
> > void
> > virCapabilitiesClearHostNUMACellCPUTopology(virCapsHostNUMACellCPUPtr cpu,
> > size_t ncpus);
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index 1c2b8f26ed..6abe15f721 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -19565,14 +19565,11
Re: [libvirt] [PATCH v1 00/26] move qemucaps validations from qemu_command to qemu_domain
On Tue, Dec 10, 2019 at 09:58:38 +, Daniel Berrange wrote: > On Mon, Dec 09, 2019 at 08:15:05PM -0300, Daniel Henrique Barboza wrote: > > (series based on master commit 97cafa610ecf5) > > > > This work was proposed by Cole in [1]. This is Cole's reasoning for > > it, copy/pasted from [1]: > > > > --- > > The benefits of moving to validate time is that XML is rejected by > > 'virsh define' rather than at 'virsh start' time. It also makes it easier > > to follow the cli building code, and makes it easier to verify > > qemu_command.c > > test suite code coverage for the important stuff like covering every CLI > > option. It's also a good intermediate step for sharing validation with > > domain capabilities building, like is done presently for rng models. > > --- > > I've not looked at the patches, but surely moving this validate from > start time, to be define time errors is going to cause functional > regressions in our ABI behaviour. > > My libvirtd daemons installs have many XML files defined which will > fail validation at various points in time, depending on what QEMU > builds I happen to have deployed. I only need them to pass the > validation when actually starting the VM. Validation is not done on the persistent or status XML files exactly for this reason. It's re-done when starting the VM so this should not be an issue. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt mdev migration, mdevctl integration
On Tue, 10 Dec 2019 10:36:36 + Daniel P. Berrangé wrote: > On Tue, Dec 10, 2019 at 11:24:44AM +0100, Cornelia Huck wrote: > > On Tue, 10 Dec 2019 10:09:34 + > > Daniel P. Berrangé wrote: > > > > > On Mon, Dec 09, 2019 at 02:23:38PM -0600, Jonathon Jongsma wrote: > > > > mdevctl also supports assigning arbitrary sysfs attributes to a device. > > > > These attributes have an explicit ordering and are written to sysfs in > > > > the specified order when a device is started. This might be the only > > > > thing that doesn't fit into the current xml format. > > > > Not sure how much the 'explicit ordering' is actually required by the > > devices currently supporting this. It's probably a good idea to keep > > this, though, as future device types might end up having such a > > requirement. > > > > > Well we need to define a schema, but there will need to be some kind > > > of validation added because. AFAICT, mdevctl does no validation, so a > > > plain passthrough of this allows arbitrary writing of files anywhere > > > on the host given a suitable malicious attribute name. > > > > Uh, we really should do something about that in mdevctl as well. Writes > > outside the sysfs hierarchy should not be allowed. > > I'm pretty worried about overall safety/reliability of the mdevctrl > tool in general. Given that it is written in shell, it is really hard > to ensure that it isn't vulnerable to any shell quoting / meta character > flaws, whether from malicious or accidental data input. I'm not sure I'm trusting myself too much to get that right, either... review obviously welcome, but this is shell, as you say. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 00/26] move qemucaps validations from qemu_command to qemu_domain
On Tue, Dec 10, 2019 at 07:22:37AM -0300, Daniel Henrique Barboza wrote: > Hi, > > I failed to make a reservation about what I've done in patch in the > cover letter. In the commit msg I mentioned about the XML files > considering SPICE support as default to ppc64, aarch64 and s390 archs and > fixing all the xmls. > > It is worth disclaiming that what I can assert to be true is the lack of > SPICE support for ppc64. I can't put my money on the lack of SPICE for > the other 2 archs - what I did in the patch was to assume that the emulator > capabilities for aarch64 and s390, that didn't report SPICE support, was > accurate. I think it's an educated guess, but a guess nevertheless. SPICE is present on arch64. 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] libvirt mdev migration, mdevctl integration
On Tue, Dec 10, 2019 at 11:24:44AM +0100, Cornelia Huck wrote: > On Tue, 10 Dec 2019 10:09:34 + > Daniel P. Berrangé wrote: > > > On Mon, Dec 09, 2019 at 02:23:38PM -0600, Jonathon Jongsma wrote: > > > mdevctl also supports assigning arbitrary sysfs attributes to a device. > > > These attributes have an explicit ordering and are written to sysfs in > > > the specified order when a device is started. This might be the only > > > thing that doesn't fit into the current xml format. > > Not sure how much the 'explicit ordering' is actually required by the > devices currently supporting this. It's probably a good idea to keep > this, though, as future device types might end up having such a > requirement. > > > Well we need to define a schema, but there will need to be some kind > > of validation added because. AFAICT, mdevctl does no validation, so a > > plain passthrough of this allows arbitrary writing of files anywhere > > on the host given a suitable malicious attribute name. > > Uh, we really should do something about that in mdevctl as well. Writes > outside the sysfs hierarchy should not be allowed. I'm pretty worried about overall safety/reliability of the mdevctrl tool in general. Given that it is written in shell, it is really hard to ensure that it isn't vulnerable to any shell quoting / meta character flaws, whether from malicious or accidental data input. 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 2/2] Add -mem-shared option
Eduardo Habkost writes: > +Markus > > On Tue, Dec 03, 2019 at 03:43:03PM +0100, Igor Mammedov wrote: >> On Tue, 3 Dec 2019 09:56:15 +0100 >> Thomas Huth wrote: >> >> > On 02/12/2019 22.00, Eduardo Habkost wrote: >> > > On Mon, Dec 02, 2019 at 08:39:48AM +0100, Igor Mammedov wrote: >> > >> On Fri, 29 Nov 2019 18:46:12 +0100 >> > >> Paolo Bonzini wrote: >> > >> >> > >>> On 29/11/19 13:16, Igor Mammedov wrote: >> > As for "-m", I'd make it just an alias that translates >> > -m/mem-path/mem-prealloc >> > >>> >> > >>> I think we should just deprecate -mem-path/-mem-prealloc in 5.0. CCing >> > >>> Thomas as mister deprecation. :) >> > >> >> > >> I'll add that to my series >> > > >> > > Considering that the plan is to eventually reimplement those >> > > options as syntactic sugar for memory backend options (hopefully >> > > in less than 2 QEMU releases), what's the point of deprecating >> > > them? >> > >> > Well, it depends on the "classification" [1] of the parameter... >> > >> > Let's ask: What's the main purpose of the option? >> > >> > Is it easier to use than the "full" option, and thus likely to be used >> > by a lot of people who run QEMU directly from the CLI? In that case it >> > should stay as "convenience option" and not be deprecated. >> > >> > Or is the option merely there to give the upper layers like libvirt or >> > some few users and their scripts some more grace period to adapt their >> > code, but we all agree that the options are rather ugly and should >> > finally go away? Then it's rather a "legacy option" and the deprecation >> > process is the right way to go. Our QEMU interface is still way >> > overcrowded, we should try to keep it as clean as possible. >> >> After switching to memdev for main RAM, users could use relatively >> short global options >> -global memory-backend.prealloc|share=on >> and >> -global memory-backend-file.mem-path=X|prealloc|share=on >> >> instead of us adding and maintaining slightly shorter >> -mem-shared/-mem-path/-mem-prealloc > > Global properties are a convenient way to expose knobs through > the command line with little effort, but we have no documentation > on which QOM properties are really supposed to be touched by > users using -global. > > Unless we fix the lack of documentation, I'd prefer to have > syntactic sugar translated to -global instead of recommending > direct usage of -global. Fair point. I'd take QOM property documentation over still more sugar. Sometimes, the practical way to make simple things simple is sugar. I can accept that. This doesn't look like such a case, though. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] qemu: directly create virResctrlInfo ignoring capabilities
On Tue, Dec 10, 2019 at 03:27:06PM +0800, Wang Huaqiang wrote: > This patch introduced a bug and broke the 'resctrl' feature. > > It introduced a 'divide by zero' error if you defined any 'resctrl' > > allocation group through . > > > Reason is 'caps->resctrl' is fully initialized through two steps, > 'virResctrlInfoNew' > > invokes 'virResctrlGetInfo' completes the first step, later, > 'virResctrlInfoGetCache' > > accomplishes the filling of > 'caps->resctrl->levels->types->control.granularity'. Urgh, that is really horribly misleading API. An object getter method should not be making changes to the object. virResctrlInfoGetCache needs splitting into two methods - one setter which updates the control granularity, and then make this getter into something with no side-effects. > The simplest way to fix the bug is drawback this patch, but still have the > undesirable overhead. I've posted the simple revert for now, since refactoring the code is a bigger job. 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
