Re: [libvirt PATCH] docs: Add pci-addresses.rst
On Tue, 14 Apr 2020 19:53:05 +0200 Andrea Bolognani wrote: > This document describes the relationship between PCI addresses as > seen in the domain XML and by the guest OS, which is a topic that > people get confused by time and time again. > > Signed-off-by: Andrea Bolognani > --- > docs/formatdomain.html.in | 6 +- > docs/pci-addresses.rst| 184 ++ > 2 files changed, 189 insertions(+), 1 deletion(-) > create mode 100644 docs/pci-addresses.rst > (...) > diff --git a/docs/pci-addresses.rst b/docs/pci-addresses.rst > new file mode 100644 > index 00..96c6466899 > --- /dev/null > +++ b/docs/pci-addresses.rst > @@ -0,0 +1,184 @@ > + > +PCI addresses in domain XML and guest OS > + > + > +.. contents:: > + > +When discussing PCI addresses, it's important to understand the the > +relationship between the addresses that can be seen in the domain XML > +and those that are visible inside the guest OS. > + > + > +Simple cases > + (...) > +More complex cases > +== (...) I'm wondering whether it is worth mentioning zPCI under 'More complex cases', or maybe under 'Completely wacky cases', as the PCI addresses a Linux guest will generate do not have any relation to whatever addresses are used in the XML at all, but only to the zPCI attributes?
Re:Re: what a correct use for virConnectDomainEventRegisterAny API, how to Obtain a stable expected result
Daniel, thanks for your help. If it cannot receive a stable expected callback, it need a timer to handle the timeout, in the timer timeout to release memory passed to virConnectDomainEventRegisterAny or, maybe memory-leak now ... At 2020-04-14 18:00:49, "Daniel P. Berrangé" wrote: >On Mon, Apr 13, 2020 at 05:32:38PM +0800, thomas.kuang wrote: >> HI, everyone: >> >> >> My target deal with network hotplug use virDomainDetachDeviceFlags. Because >> when the API return ,the network maybe doesn’t remove from my vm guest os. >> >> So I use virConnectDomainEventRegisterAny to register an event ID: >> VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED , >> >> my process as follow: >> >> cb_para->call_id=virConnectDomainEventRegisterAny(cb_para->conn,cb_para->dom,VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED, >> VIR_DOMAIN_EVENT_CALLBACK(vnf_control_del_network_cb), cb_para, >> vnf_control_del_network_cb_free); >> >> >> >> flags |= VIR_DOMAIN_AFFECT_CONFIG; >> if (virDomainIsActive(dom) == 1) { >> flags |= VIR_DOMAIN_AFFECT_LIVE; >> } >> >> ret = virDomainDetachDeviceFlags(dom, xml, flags); >> >> >> >> >> above code write in thread loop ,then in the same loop : >> >> >> >>while (1) { >> >> mission = vnf_mission_queue_get(task); >> >> if (mission == NULL) { >> >> sleep(1); >> >> continue; >> >> } >> >> vnf_op_process(&mission->info); // this will deal with network >> hotplug,will call virConnectDomainEventRegisterAny then call >> virDomainDetachDeviceFlags >> >> if (mission) { >> >> vnf_mission_free(mission); >> >> } >> >> if(virEventRunDefaultImpl() < 0) { // at here process the >> registered callback for event-registered >> >> printf(); >> >> >> } >> >>} >> >> My problem is: some time , the virEventRunDefaultImpl can trigger the >> vnf_control_del_network_cb callback ,but some time there is nothing, >> as if the VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED has lost. >> what cause the Unpredictable behavior ? what is a correct use for >> virConnectDomainEventRegisterAny ? > >The virDomainDetachDeviceFlags API is asynchronous and requires a cooperative >guest to complete. It merely injects an ACPI PCI unplug request to the guest >OS. If the guest OS doesn't honour this request (because it is in early >bootup, or is in BIOS, or is crashed, or is configured to disable hotplug, >or is maliciously not responding), then the device will never be unplugged, >and so you'll never get an event. > >So the first thing for you todo is to validate that the device really is >being unplugged in the guest OS, and in QEMU. > >To check QEMU run: > > virsh qemu-monitor-command --hmp $GUEST "info pci" > >and look to see if the PCI device is still listed in the output. > >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 :|
Re: [PATCH] libxl: enable Xen's e820_host setting
On 4/14/20 4:07 PM, Marek Marczykowski-Górecki wrote: On Tue, Apr 14, 2020 at 03:56:47PM -0600, Jim Fehlig wrote: On 4/13/20 1:17 PM, Marek Marczykowski-Górecki wrote: FWIW, in Qubes we have a patches adding e820_host setting here: https://github.com/QubesOS/qubes-core-libvirt/ (patches 8-11) Not submitted before, exactly to avoid adding temporary options. But since 8+ years later it is still there, I think it's safe to assume it will be there for some more. Or at least it's worth to unbreak some configurations in the meantime. BTW, the other piece of the puzzle for PCI passthrough starting with xen 4.13 is the xl.cfg 'passthrough' option. See xen commit babde47a3fe. Without that enabled, hotplugging a PCI device to an HVM or PV domain is not possible, with or without e820_host. AFAICT there are no patches in your queue for the 'passthrough' option. Perhaps you haven't rebased to xen 4.13. TBH, I don't do PCI hotplugging. Lucky you :-). Do you have any thoughts on modeling that option? My thoughts are adding another xen hypervisor feature (similar to e820_host) or map it to the new IOMMU device in libvirt [1]. I suppose it depends on the need to support the {sync,share}_pt values vs a simple enable/disable. If enabling and disabling passthrough is enough, I'd lean towards another xen hypervisor feature. Otherwise we'll need to explore the IOMMU device? Isn't IOMMU device about allowing the guest to control (v)IOMMU? I've seen some discussion about it in context of Xen, so IMO we should not use it now for something else. What about adding ? I'm not sure about {sync,share}_pt, but probably could be squeezed there if needed. I suppose adding a PCI controller is one way of configuring the guest to later hotplug PCI devices, although I'm not sure if (sync,share}_pt could be considered attributes of a PCI controller. Also, xen has no notion of a PCI controller that I'm aware of, so it would only exist in libvirt. It seems like a heavy-weight approach for a setting that essentially enables or disables PCI passthrough. Daniel, do you have any ideas about modeling the xl.cfg(5) 'passthrough' option? For convenience, see 'passthrough' under "Other Options" https://xenbits.xen.org/docs/unstable/man/xl.cfg.5.html#Other-Options Regards, Jim
Re: [PATCH] libxl: enable Xen's e820_host setting
On Tue, Apr 14, 2020 at 03:56:47PM -0600, Jim Fehlig wrote: > On 4/13/20 1:17 PM, Marek Marczykowski-Górecki wrote: > > FWIW, in Qubes we have a patches adding e820_host setting here: > > https://github.com/QubesOS/qubes-core-libvirt/ > > (patches 8-11) > > Not submitted before, exactly to avoid adding temporary options. But > > since 8+ years later it is still there, I think it's safe to assume it > > will be there for some more. Or at least it's worth to unbreak some > > configurations in the meantime. > > BTW, the other piece of the puzzle for PCI passthrough starting with xen > 4.13 is the xl.cfg 'passthrough' option. See xen commit babde47a3fe. Without > that enabled, hotplugging a PCI device to an HVM or PV domain is not > possible, with or without e820_host. AFAICT there are no patches in your > queue for the 'passthrough' option. Perhaps you haven't rebased to xen 4.13. TBH, I don't do PCI hotplugging. > Do you have any thoughts on modeling that option? > > My thoughts are adding another xen hypervisor feature (similar to e820_host) > or map it to the new IOMMU device in libvirt [1]. I suppose it depends on > the need to support the {sync,share}_pt values vs a simple enable/disable. > If enabling and disabling passthrough is enough, I'd lean towards another > xen hypervisor feature. Otherwise we'll need to explore the IOMMU device? Isn't IOMMU device about allowing the guest to control (v)IOMMU? I've seen some discussion about it in context of Xen, so IMO we should not use it now for something else. What about adding ? I'm not sure about {sync,share}_pt, but probably could be squeezed there if needed. > Regards, > Jim > > [1] https://libvirt.org/formatdomain.html#elementsIommu -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? signature.asc Description: PGP signature
Re: [PATCH v2 1/4] conf: add xen specific feature: e820_host
On 4/13/20 8:37 PM, Marek Marczykowski-Górecki wrote: e820_host is a Xen-specific option, only available for PV domains, that provides the domain a virtual e820 memory map based on the host one. It is required when using PCI passthrough and is generally considered safe for any PV kernel. e820_host is silently ignored if set in HVM domain configuration. See xl.cfg(5) man page in the Xen documentation for more details. Signed-off-by: Marek Marczykowski-Górecki This looks good to me now so Reviewed-by: Jim Fehlig But before pushing I'd like to settle on mapping the new 'passthrough' xl.cfg option to libvirt. See https://www.redhat.com/archives/libvir-list/2020-April/msg00696.html On xen 4.13 it seems 'passthrough' is enough for PV domains. Enabling it, and not e820_host, allows me to successfully hotplug a PCI device after PV domain creation. I haven't checked but I assume e820_host was needed to hotplug PCI devices with xen < 4.13? Regards, Jim
Re: [PATCH] libxl: enable Xen's e820_host setting
On 4/13/20 1:17 PM, Marek Marczykowski-Górecki wrote: FWIW, in Qubes we have a patches adding e820_host setting here: https://github.com/QubesOS/qubes-core-libvirt/ (patches 8-11) Not submitted before, exactly to avoid adding temporary options. But since 8+ years later it is still there, I think it's safe to assume it will be there for some more. Or at least it's worth to unbreak some configurations in the meantime. BTW, the other piece of the puzzle for PCI passthrough starting with xen 4.13 is the xl.cfg 'passthrough' option. See xen commit babde47a3fe. Without that enabled, hotplugging a PCI device to an HVM or PV domain is not possible, with or without e820_host. AFAICT there are no patches in your queue for the 'passthrough' option. Perhaps you haven't rebased to xen 4.13. Do you have any thoughts on modeling that option? My thoughts are adding another xen hypervisor feature (similar to e820_host) or map it to the new IOMMU device in libvirt [1]. I suppose it depends on the need to support the {sync,share}_pt values vs a simple enable/disable. If enabling and disabling passthrough is enough, I'd lean towards another xen hypervisor feature. Otherwise we'll need to explore the IOMMU device? Regards, Jim [1] https://libvirt.org/formatdomain.html#elementsIommu
Re: [PATCH 1/2] docs: backup: Remove references to push backup to network disk
On 4/14/20 2:06 PM, Peter Krempa wrote: On Tue, Apr 14, 2020 at 12:36:56 -0500, Eric Blake wrote: On 4/14/20 4:22 AM, Peter Krempa wrote: It was never implemented and for now I don't think there's demand to do it. Remove the reference. https://bugzilla.redhat.com/show_bug.cgi?id=1812100 Signed-off-by: Peter Krempa --- docs/formatbackup.html.in | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/formatbackup.html.in b/docs/formatbackup.html.in index 55acd13ddc..87744bac98 100644 --- a/docs/formatbackup.html.in +++ b/docs/formatbackup.html.in @@ -97,12 +97,11 @@ type A mandatory attribute to describe the type of the disk, except when backup='no' is -used. Valid values include file, -block, or network. +used. Valid values include file, or +block. I think we should implement block, rather than delete it. It matters for the same reason that it matters in the destination of block copy: if you I'm deleting 'network'. Block is implemented, working and tested. Then shame on me for misreading the patch. Okay, we may someday add network, but for now making the documentation match what we have as existing implementation makes sense. want to set a highest-byte watermark threshold (to be warned by qemu when it is time to resize the disk larger), you NEED a block device, not a file. You can do this on a file too. I seem to recall differences in how the two behave at the qemu level; but it doesn't affect this patch if we have the means for telling libvirt which of the two (file or block) we meant. I'm inclined to NACK this patch. Wouldn't mean that much since it's necessary to add schema if you want it. NACK withdrawn; you've convinced me that the patch (dropping 'network', not 'block') is reasonable for matching what we have. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [libvirt PATCH] docs: Add pci-addresses.rst
On 4/14/20 1:53 PM, Andrea Bolognani wrote: This document describes the relationship between PCI addresses as seen in the domain XML and by the guest OS, which is a topic that people get confused by time and time again. Signed-off-by: Andrea Bolognani --- docs/formatdomain.html.in | 6 +- docs/pci-addresses.rst| 184 ++ 2 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 docs/pci-addresses.rst diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6f43976815..0077666862 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4286,7 +4286,11 @@ element with no other attributes as an explicit request to assign a PCI address for the device rather than some other type of address that may also be appropriate for that same -device (e.g. virtio-mmio). +device (e.g. virtio-mmio). +The relationship between the PCI addresses configured in the domain +XML and those seen by the guest OS can sometime seem confusing: a +separate document describes how PCI +addresses work in more detail. drive Drive addresses have the following additional diff --git a/docs/pci-addresses.rst b/docs/pci-addresses.rst new file mode 100644 index 00..96c6466899 --- /dev/null +++ b/docs/pci-addresses.rst @@ -0,0 +1,184 @@ + +PCI addresses in domain XML and guest OS + + +.. contents:: + +When discussing PCI addresses, it's important to understand the the +relationship between the addresses that can be seen in the domain XML +and those that are visible inside the guest OS. + + +Simple cases + + +When the PCI topology of the VM is very simple, the PCI addresses +will usually match. + +For example, the domain XML snippet + +:: + + + + + + + + + + + + + +will result in the PCI topology + +:: + + :00:00.0 Host bridge: Intel Corporation 82G33/G31/P35/P31 Express DRAM Controller + :00:01.0 PCI bridge: Red Hat, Inc. QEMU PCIe Root port + :01:00.0 Ethernet controller: Red Hat, Inc. Virtio network device (rev 01) + +showing up in the guest OS. + +The PCI address of the ``virtio-net`` adapter, ``:01:00.0``, is +the same in both cases, so there's no confusion. + + +More complex cases +== + +In more complex cases, the PCI address visible in the domain XML will +correlate to the one seen by the guest OS in a less obvious way. + +pcie-expander-bus +- + +This fairly uncommon device, which can be used with ``x86_64/q35`` +guests, will help illustrate one such scenario. + +For example, the domain XML snippet + +:: + + + + + + + + + + + + + + + + + + +will result in the PCI topology + +:: + + :00:00.0 Host bridge: Intel Corporation 82G33/G31/P35/P31 Express DRAM Controller + :00:01.0 Host bridge: Red Hat, Inc. QEMU PCIe Expander bridge + :fe:00.0 PCI bridge: Red Hat, Inc. QEMU PCIe Root port + :ff:00.0 Ethernet controller: Red Hat, Inc. Virtio network device (rev 01) + +showing up in the guest OS. + +This time the addresses don't match: this is because the ``busNr`` +property for the ``pcie-expander-bus`` controller causes it to show +up as bus 254 (``0xfe`` in hexadecimal) instead of bus 1 as one might +expect based on its ``index`` property. + +How can the domain XML shown above work at all, then? Surely the +``pcie-root-port`` controller and the ``virtio-net`` adapter should +use ``bus=0xfe`` and ``bus=0xff`` respectively for the configuration +to be accepted by libvirt? + +As it turns out, that's not the case. The reason for this is that +QEMU, and consequently libvirt, uses the ``bus`` property of a +device's PCI address only to match it with the PCI controller that +has the same ``index`` property, and not to set the actual PCI +address, which is decided by the guest OS. + +So, by looking at the XML snippet above, we can see that the +``virtio-net`` adapter plugs into the ``pcie-root-port`` controller, +which plugs into the ``pcie-expander-bus`` controller, which plugs +into ``pcie-root``: the guest OS sees the same topology, but assigns +different PCI addresses to some of its component. + +The takeaway is that the *relationship* between controllers are the +very same whether you look at the domain XML or at the guest OS, but +the *actual PCI addresses* are not guaranteed to match and in fact, +except for the very simplest cases, they usually will not. and it doesn't necessarily take a pcie-expander-bus to make the numbering appear "off". It really is 100% up to the guest OS what bus number is given to each bus that it discovers, and there's nothing the host can do about it. This is similar to the target device name for block devices - you can put in the config for a disk as much as you w
Re: [PATCH 1/2] docs: backup: Remove references to push backup to network disk
On Tue, Apr 14, 2020 at 12:36:56 -0500, Eric Blake wrote: > On 4/14/20 4:22 AM, Peter Krempa wrote: > > It was never implemented and for now I don't think there's demand to do > > it. Remove the reference. > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1812100 > > > > Signed-off-by: Peter Krempa > > --- > > docs/formatbackup.html.in | 7 +++ > > 1 file changed, 3 insertions(+), 4 deletions(-) > > > > diff --git a/docs/formatbackup.html.in b/docs/formatbackup.html.in > > index 55acd13ddc..87744bac98 100644 > > --- a/docs/formatbackup.html.in > > +++ b/docs/formatbackup.html.in > > @@ -97,12 +97,11 @@ > > type > > A mandatory attribute to describe the type of the > > disk, except when backup='no' is > > -used. Valid values include file, > > -block, or network. > > +used. Valid values include file, or > > +block. > > I think we should implement block, rather than delete it. It matters for > the same reason that it matters in the destination of block copy: if you I'm deleting 'network'. Block is implemented, working and tested. > want to set a highest-byte watermark threshold (to be warned by qemu when it > is time to resize the disk larger), you NEED a block device, not a file. You can do this on a file too. > But libvirt treats all as files, even when opening > /path/to/device; you HAVE to use when specifying a block > device to get the behaviors needed for handling it as a block device rather > than a file. > > > Similar to a disk declaration for a domain, the choice of > > type > > controls what additional sub-elements are needed to > > describe > > -the destination (such as protocol for a > > -network destination). > > +the destination. > > target > > Valid only for push mode backups, this is the > > primary sub-element that describes the file name of > > > > I'm inclined to NACK this patch. Wouldn't mean that much since it's necessary to add schema if you want it. > > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3226 > Virtualization: qemu.org | libvirt.org
Re: [RFC] Adding docker driver to libvirt
On Tue, 2020-04-14 at 18:34 +0200, Martin Kletzander wrote: > On Tue, Apr 14, 2020 at 09:56:24AM +0300, nshirokovskiy wrote: > > We wanted to add Windows containers to the libvirt API. They are available > > under docker API thus the idea to add a docker driver. The docker itself > > uses some API to manage Windows containers but this API lacks documentation > > thus again the willingness to use just docker API to bring Windows > > containers > > to libvirt. > > Oh, so with that we would be able to manage native containers on Windows? > That > might be interesting then. I did not know about that because last time I > heard > about containers on Windows docker was running a Linux VM in which it spawned > the containers =D I did not know there's something new now. Note that we don't build the daemon on Windows, so this would have to be a stateless driver. I think that's probably fine, since Docker already has its own daemon which supports remote connections (see DOCKER_HOST in [1]). [1] https://docs.docker.com/engine/reference/commandline/cli/#environment-variables -- Andrea Bolognani / Red Hat / Virtualization
[libvirt PATCH] docs: Add pci-addresses.rst
This document describes the relationship between PCI addresses as seen in the domain XML and by the guest OS, which is a topic that people get confused by time and time again. Signed-off-by: Andrea Bolognani --- docs/formatdomain.html.in | 6 +- docs/pci-addresses.rst| 184 ++ 2 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 docs/pci-addresses.rst diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 6f43976815..0077666862 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4286,7 +4286,11 @@ element with no other attributes as an explicit request to assign a PCI address for the device rather than some other type of address that may also be appropriate for that same -device (e.g. virtio-mmio). +device (e.g. virtio-mmio). +The relationship between the PCI addresses configured in the domain +XML and those seen by the guest OS can sometime seem confusing: a +separate document describes how PCI +addresses work in more detail. drive Drive addresses have the following additional diff --git a/docs/pci-addresses.rst b/docs/pci-addresses.rst new file mode 100644 index 00..96c6466899 --- /dev/null +++ b/docs/pci-addresses.rst @@ -0,0 +1,184 @@ + +PCI addresses in domain XML and guest OS + + +.. contents:: + +When discussing PCI addresses, it's important to understand the the +relationship between the addresses that can be seen in the domain XML +and those that are visible inside the guest OS. + + +Simple cases + + +When the PCI topology of the VM is very simple, the PCI addresses +will usually match. + +For example, the domain XML snippet + +:: + + + + + + + + + + + + + +will result in the PCI topology + +:: + + :00:00.0 Host bridge: Intel Corporation 82G33/G31/P35/P31 Express DRAM Controller + :00:01.0 PCI bridge: Red Hat, Inc. QEMU PCIe Root port + :01:00.0 Ethernet controller: Red Hat, Inc. Virtio network device (rev 01) + +showing up in the guest OS. + +The PCI address of the ``virtio-net`` adapter, ``:01:00.0``, is +the same in both cases, so there's no confusion. + + +More complex cases +== + +In more complex cases, the PCI address visible in the domain XML will +correlate to the one seen by the guest OS in a less obvious way. + +pcie-expander-bus +- + +This fairly uncommon device, which can be used with ``x86_64/q35`` +guests, will help illustrate one such scenario. + +For example, the domain XML snippet + +:: + + + + + + + + + + + + + + + + + + +will result in the PCI topology + +:: + + :00:00.0 Host bridge: Intel Corporation 82G33/G31/P35/P31 Express DRAM Controller + :00:01.0 Host bridge: Red Hat, Inc. QEMU PCIe Expander bridge + :fe:00.0 PCI bridge: Red Hat, Inc. QEMU PCIe Root port + :ff:00.0 Ethernet controller: Red Hat, Inc. Virtio network device (rev 01) + +showing up in the guest OS. + +This time the addresses don't match: this is because the ``busNr`` +property for the ``pcie-expander-bus`` controller causes it to show +up as bus 254 (``0xfe`` in hexadecimal) instead of bus 1 as one might +expect based on its ``index`` property. + +How can the domain XML shown above work at all, then? Surely the +``pcie-root-port`` controller and the ``virtio-net`` adapter should +use ``bus=0xfe`` and ``bus=0xff`` respectively for the configuration +to be accepted by libvirt? + +As it turns out, that's not the case. The reason for this is that +QEMU, and consequently libvirt, uses the ``bus`` property of a +device's PCI address only to match it with the PCI controller that +has the same ``index`` property, and not to set the actual PCI +address, which is decided by the guest OS. + +So, by looking at the XML snippet above, we can see that the +``virtio-net`` adapter plugs into the ``pcie-root-port`` controller, +which plugs into the ``pcie-expander-bus`` controller, which plugs +into ``pcie-root``: the guest OS sees the same topology, but assigns +different PCI addresses to some of its component. + +The takeaway is that the *relationship* between controllers are the +very same whether you look at the domain XML or at the guest OS, but +the *actual PCI addresses* are not guaranteed to match and in fact, +except for the very simplest cases, they usually will not. + +spapr-pci-host-bridge +- + +This device, which is unique to ``ppc64/pseries`` guests, will help +illustrate another scenario. + +For example, the domain XML snippet + +:: + + + + + + + + + + + + + + + +will result in the PCI topology + +:: + + 0001:00:01.0 Ethernet controller: Red Hat, Inc. Virtio network device + +showing up in
Re: [libvirt PATCH 0/2] gitlab: Clean up and improve ccache usage
On Mon, 2020-03-30 at 19:31 +0200, Andrea Bolognani wrote: > Test pipeline: https://gitlab.com/abologna/libvirt/pipelines/131128028 > > Andrea Bolognani (2): > gitlab: Don't define $MAKE > gitlab: Enable improved ccache usage > > .gitlab-ci.yml | 13 - > 1 file changed, 4 insertions(+), 9 deletions(-) alias ping="ccache ping" -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH 1/2] docs: backup: Remove references to push backup to network disk
On 4/14/20 4:22 AM, Peter Krempa wrote: It was never implemented and for now I don't think there's demand to do it. Remove the reference. https://bugzilla.redhat.com/show_bug.cgi?id=1812100 Signed-off-by: Peter Krempa --- docs/formatbackup.html.in | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/formatbackup.html.in b/docs/formatbackup.html.in index 55acd13ddc..87744bac98 100644 --- a/docs/formatbackup.html.in +++ b/docs/formatbackup.html.in @@ -97,12 +97,11 @@ type A mandatory attribute to describe the type of the disk, except when backup='no' is -used. Valid values include file, -block, or network. +used. Valid values include file, or +block. I think we should implement block, rather than delete it. It matters for the same reason that it matters in the destination of block copy: if you want to set a highest-byte watermark threshold (to be warned by qemu when it is time to resize the disk larger), you NEED a block device, not a file. But libvirt treats all as files, even when opening /path/to/device; you HAVE to use when specifying a block device to get the behaviors needed for handling it as a block device rather than a file. Similar to a disk declaration for a domain, the choice of type controls what additional sub-elements are needed to describe -the destination (such as protocol for a -network destination). +the destination. target Valid only for push mode backups, this is the primary sub-element that describes the file name of I'm inclined to NACK this patch. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH 02/43] bhyve: convert virMutex to GMutex
On Tue, Apr 14, 2020 at 6:06 PM Pavel Mores wrote: > > By the way, the approach taken here with bhyveDriver{Lock,Unlock}() might make > sense with the whole series - implement e.g. virMutexInit() in terms of > g_mutex_init() in the first phase and only then replace the actual > virMutexInit() calls if considered beneficial... So you mean one patch doing 's/virMutex/GMutex' and then inside virMutex*() we call the g_mutex_*() equivalent? And maybe make virMutex*() `inline`? Att. -- Rafael Fonseca
Re: [PATCH 20/43] network: bridge_driver: convert virMutex to GMutex
On Fri, Apr 10, 2020 at 03:54:47PM +0200, Rafael Fonseca wrote: > Signed-off-by: Rafael Fonseca > --- > src/network/bridge_driver.c | 11 --- > src/network/bridge_driver_platform.h | 2 +- > 2 files changed, 5 insertions(+), 8 deletions(-) > > diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c > index f06099297a..2eaab9c667 100644 > --- a/src/network/bridge_driver.c > +++ b/src/network/bridge_driver.c > @@ -101,14 +101,14 @@ networkGetDriver(void) > static void > networkDriverLock(virNetworkDriverStatePtr driver) > { > -virMutexLock(&driver->lock); > +g_mutex_lock(&driver->lock); > } > > > static void > networkDriverUnlock(virNetworkDriverStatePtr driver) > { > -virMutexUnlock(&driver->lock); > +g_mutex_unlock(&driver->lock); > } > > > @@ -726,10 +726,7 @@ networkStateInitialize(bool privileged, > goto error; > > network_driver->lockFD = -1; > -if (virMutexInit(&network_driver->lock) < 0) { > -VIR_FREE(network_driver); > -goto error; > -} > +g_mutex_init(&network_driver->lock); > > network_driver->privileged = privileged; > > @@ -907,7 +904,7 @@ networkStateCleanup(void) > > virObjectUnref(network_driver->dnsmasqCaps); > > -virMutexDestroy(&network_driver->lock); > +g_mutex_clear(&network_driver->lock); > > VIR_FREE(network_driver); > > diff --git a/src/network/bridge_driver_platform.h > b/src/network/bridge_driver_platform.h > index 169417a6c0..6528bf6647 100644 > --- a/src/network/bridge_driver_platform.h > +++ b/src/network/bridge_driver_platform.h > @@ -29,7 +29,7 @@ > > /* Main driver state */ > struct _virNetworkDriverState { > -virMutex lock; > +GMutex lock; > > /* Read-only */ > bool privileged; > -- > 2.25.2 > > Reviewed-by: Pavel Mores
Re: [PATCH 06/43] util: virtpm: convert virMutex to GMutex
On Fri, Apr 10, 2020 at 03:54:33PM +0200, Rafael Fonseca wrote: > Signed-off-by: Rafael Fonseca > --- > src/util/virtpm.c | 18 +- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/src/util/virtpm.c b/src/util/virtpm.c > index c734bf941a..5fd6396f2f 100644 > --- a/src/util/virtpm.c > +++ b/src/util/virtpm.c > @@ -91,7 +91,7 @@ virTPMCreateCancelPath(const char *devpath) > * executables for the swtpm; to be found on the host along with > * capabilties bitmap > */ > -static virMutex swtpm_tools_lock = VIR_MUTEX_INITIALIZER; > +G_LOCK_DEFINE_STATIC(swtpm_tools_lock); > static char *swtpm_path; > static struct stat swtpm_stat; > static virBitmapPtr swtpm_caps; > @@ -113,9 +113,9 @@ virTPMGetSwtpm(void) > if (!swtpm_path && virTPMEmulatorInit() < 0) > return NULL; > > -virMutexLock(&swtpm_tools_lock); > +G_LOCK(swtpm_tools_lock); > s = g_strdup(swtpm_path); > -virMutexUnlock(&swtpm_tools_lock); > +G_UNLOCK(swtpm_tools_lock); > > return s; > } > @@ -128,9 +128,9 @@ virTPMGetSwtpmSetup(void) > if (!swtpm_setup && virTPMEmulatorInit() < 0) > return NULL; > > -virMutexLock(&swtpm_tools_lock); > +G_LOCK(swtpm_tools_lock); > s = g_strdup(swtpm_setup); > -virMutexUnlock(&swtpm_tools_lock); > +G_UNLOCK(swtpm_tools_lock); > > return s; > } > @@ -143,9 +143,9 @@ virTPMGetSwtpmIoctl(void) > if (!swtpm_ioctl && virTPMEmulatorInit() < 0) > return NULL; > > -virMutexLock(&swtpm_tools_lock); > +G_LOCK(swtpm_tools_lock); > s = g_strdup(swtpm_ioctl); > -virMutexUnlock(&swtpm_tools_lock); > +G_UNLOCK(swtpm_tools_lock); > > return s; > } > @@ -284,7 +284,7 @@ virTPMEmulatorInit(void) > }; > size_t i; > > -virMutexLock(&swtpm_tools_lock); > +G_LOCK(swtpm_tools_lock); > > for (i = 0; i < G_N_ELEMENTS(prgs); i++) { > g_autofree char *path = NULL; > @@ -341,7 +341,7 @@ virTPMEmulatorInit(void) > ret = 0; > > cleanup: > -virMutexUnlock(&swtpm_tools_lock); > +G_UNLOCK(swtpm_tools_lock); > > return ret; > } > -- > 2.25.2 > > Reviewed-by: Pavel Mores
Re: [PATCH 04/43] conf: virchrdev: convert virMutex to GMutex
On Fri, Apr 10, 2020 at 03:54:31PM +0200, Rafael Fonseca wrote: > Signed-off-by: Rafael Fonseca > --- > src/conf/virchrdev.c | 27 +++ > 1 file changed, 7 insertions(+), 20 deletions(-) > > diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c > index 800e82869e..8280f8e188 100644 > --- a/src/conf/virchrdev.c > +++ b/src/conf/virchrdev.c > @@ -44,7 +44,7 @@ VIR_LOG_INIT("conf.chrdev"); > /* structure holding information about character devices > * open in a given domain */ > struct _virChrdevs { > -virMutex lock; > +GMutex lock; > virHashTablePtr hash; > }; > > @@ -238,12 +238,10 @@ static void virChrdevFDStreamCloseCb(virStreamPtr st > G_GNUC_UNUSED, >void *opaque) > { > virChrdevStreamInfoPtr priv = opaque; > -virMutexLock(&priv->devs->lock); > +g_autoptr(GMutexLocker) locker = g_mutex_locker_new(&priv->devs->lock); > > /* remove entry from hash */ > virHashRemoveEntry(priv->devs->hash, priv->path); > - > -virMutexUnlock(&priv->devs->lock); > } > > /** > @@ -258,12 +256,7 @@ virChrdevsPtr virChrdevAlloc(void) > if (VIR_ALLOC(devs) < 0) > return NULL; > > -if (virMutexInit(&devs->lock) < 0) { > -virReportSystemError(errno, "%s", > - _("Unable to init device stream mutex")); > -VIR_FREE(devs); > -return NULL; > -} > +g_mutex_init(&devs->lock); > > /* there will hardly be any devices most of the time, the hash > * does not have to be huge */ > @@ -299,11 +292,11 @@ void virChrdevFree(virChrdevsPtr devs) > if (!devs) > return; > > -virMutexLock(&devs->lock); > +g_mutex_lock(&devs->lock); > virHashForEach(devs->hash, virChrdevFreeClearCallbacks, NULL); > virHashFree(devs->hash); > -virMutexUnlock(&devs->lock); > -virMutexDestroy(&devs->lock); > +g_mutex_unlock(&devs->lock); > +g_mutex_clear(&devs->lock); > > VIR_FREE(devs); > } > @@ -334,6 +327,7 @@ int virChrdevOpen(virChrdevsPtr devs, > char *path; > int ret; > bool added = false; > +g_autoptr(GMutexLocker) locker = g_mutex_locker_new(&devs->lock); > > switch (source->type) { > case VIR_DOMAIN_CHR_TYPE_PTY: > @@ -354,12 +348,9 @@ int virChrdevOpen(virChrdevsPtr devs, > return -1; > } > > -virMutexLock(&devs->lock); > - > if ((ent = virHashLookup(devs->hash, path))) { > if (!force) { > /* entry found, device is busy */ > -virMutexUnlock(&devs->lock); > return 1; > } else { > /* terminate existing connection */ > @@ -378,13 +369,11 @@ int virChrdevOpen(virChrdevsPtr devs, > > /* create the lock file */ > if ((ret = virChrdevLockFileCreate(path)) < 0) { > -virMutexUnlock(&devs->lock); > return ret; > } > > /* obtain a reference to the stream */ > if (virStreamRef(st) < 0) { > -virMutexUnlock(&devs->lock); > return -1; > } > > @@ -428,7 +417,6 @@ int virChrdevOpen(virChrdevsPtr devs, >cbdata, >virChrdevFDStreamCloseCbFree); > > -virMutexUnlock(&devs->lock); > return 0; > > error: > @@ -440,7 +428,6 @@ int virChrdevOpen(virChrdevsPtr devs, > if (cbdata) > VIR_FREE(cbdata->path); > VIR_FREE(cbdata); > -virMutexUnlock(&devs->lock); > virChrdevHashEntryFree(ent); > return -1; > } > -- > 2.25.2 > > Reviewed-by: Pavel Mores
Re: [RFC] Adding docker driver to libvirt
On Tue, Apr 14, 2020 at 09:56:24AM +0300, nshirokovskiy wrote: On 12.04.2020 12:39, Martin Kletzander wrote: On Thu, Apr 09, 2020 at 03:30:11PM +0300, nshirokovskiy wrote: Hi, all. Does it make sense to add such a driver? I can't say I have a big picture of docker functionality in mind but at least container lifecycle management and container networking are common to both. I think we had something in virt-tools that was able to pull an image from docker hub and run it with lxc. Or was it part of sandbox? I don't know. Anyway, what would be the benefit of that? We wanted to add Windows containers to the libvirt API. They are available under docker API thus the idea to add a docker driver. The docker itself uses some API to manage Windows containers but this API lacks documentation thus again the willingness to use just docker API to bring Windows containers to libvirt. Oh, so with that we would be able to manage native containers on Windows? That might be interesting then. I did not know about that because last time I heard about containers on Windows docker was running a Linux VM in which it spawned the containers =D I did not know there's something new now. Nikolay signature.asc Description: PGP signature
Re: [PATCH 02/43] bhyve: convert virMutex to GMutex
On Fri, Apr 10, 2020 at 03:54:29PM +0200, Rafael Fonseca wrote: > Signed-off-by: Rafael Fonseca > --- > src/bhyve/bhyve_driver.c | 11 --- > src/bhyve/bhyve_utils.h | 2 +- > 2 files changed, 5 insertions(+), 8 deletions(-) > > diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c > index b6204c7fb9..6e9a79cb52 100644 > --- a/src/bhyve/bhyve_driver.c > +++ b/src/bhyve/bhyve_driver.c > @@ -73,13 +73,13 @@ bhyveConnPtr bhyve_driver = NULL; > void > bhyveDriverLock(bhyveConnPtr driver) > { > -virMutexLock(&driver->lock); > +g_mutex_lock(&driver->lock); > } > > void > bhyveDriverUnlock(bhyveConnPtr driver) > { > -virMutexUnlock(&driver->lock); > +g_mutex_unlock(&driver->lock); > } > > static int > @@ -1199,7 +1199,7 @@ bhyveStateCleanup(void) > if (bhyve_driver->lockFD != -1) > virPidFileRelease(BHYVE_STATE_DIR, "driver", bhyve_driver->lockFD); > > -virMutexDestroy(&bhyve_driver->lock); > +g_mutex_clear(&bhyve_driver->lock); > VIR_FREE(bhyve_driver); > > return 0; > @@ -1228,10 +1228,7 @@ bhyveStateInitialize(bool privileged, > return VIR_DRV_STATE_INIT_ERROR; > > bhyve_driver->lockFD = -1; > -if (virMutexInit(&bhyve_driver->lock) < 0) { > -VIR_FREE(bhyve_driver); > -return VIR_DRV_STATE_INIT_ERROR; > -} > +g_mutex_init(&bhyve_driver->lock); > > if (!(bhyve_driver->closeCallbacks = virCloseCallbacksNew())) > goto cleanup; > diff --git a/src/bhyve/bhyve_utils.h b/src/bhyve/bhyve_utils.h > index f3e80b6121..a92ecb48c4 100644 > --- a/src/bhyve/bhyve_utils.h > +++ b/src/bhyve/bhyve_utils.h > @@ -44,7 +44,7 @@ struct _virBhyveDriverConfig { > }; > > struct _bhyveConn { > -virMutex lock; > +GMutex lock; > > virBhyveDriverConfigPtr config; > > -- > 2.25.2 > > By the way, the approach taken here with bhyveDriver{Lock,Unlock}() might make sense with the whole series - implement e.g. virMutexInit() in terms of g_mutex_init() in the first phase and only then replace the actual virMutexInit() calls if considered beneficial... Reviewed-by: Pavel Mores
Re: [PATCH 01/43] admin: convert virMutex to GMutex
On Fri, Apr 10, 2020 at 03:54:28PM +0200, Rafael Fonseca wrote: > Signed-off-by: Rafael Fonseca > --- > src/admin/admin_server_dispatch.c | 13 - > 1 file changed, 4 insertions(+), 9 deletions(-) > > diff --git a/src/admin/admin_server_dispatch.c > b/src/admin/admin_server_dispatch.c > index b3da577995..2515528779 100644 > --- a/src/admin/admin_server_dispatch.c > +++ b/src/admin/admin_server_dispatch.c > @@ -45,7 +45,7 @@ typedef daemonAdmClientPrivate *daemonAdmClientPrivatePtr; > /* Separate private data for admin connection */ > struct daemonAdmClientPrivate { > /* Just a placeholder, not that there is anything to be locked */ > -virMutex lock; > +GMutex lock; > > virNetDaemonPtr dmn; > }; > @@ -55,7 +55,7 @@ remoteAdmClientFree(void *data) > { > struct daemonAdmClientPrivate *priv = data; > > -virMutexDestroy(&priv->lock); > +g_mutex_clear(&priv->lock); > virObjectUnref(priv->dmn); > VIR_FREE(priv); > } > @@ -91,11 +91,7 @@ remoteAdmClientNew(virNetServerClientPtr client > G_GNUC_UNUSED, > if (VIR_ALLOC(priv) < 0) > return NULL; > > -if (virMutexInit(&priv->lock) < 0) { > -VIR_FREE(priv); > -virReportSystemError(errno, "%s", _("unable to init mutex")); > -return NULL; > -} > +g_mutex_init(&priv->lock); > > /* > * We don't necessarily need to ref this object right now as there > @@ -167,9 +163,9 @@ adminDispatchConnectOpen(virNetServerPtr server > G_GNUC_UNUSED, > struct daemonAdmClientPrivate *priv = > virNetServerClientGetPrivateData(client); > int ret = -1; > +g_autoptr(GMutexLocker) locker = g_mutex_locker_new(&priv->lock); > > VIR_DEBUG("priv=%p dmn=%p", priv, priv->dmn); > -virMutexLock(&priv->lock); > > flags = args->flags; > virCheckFlagsGoto(0, cleanup); > @@ -178,7 +174,6 @@ adminDispatchConnectOpen(virNetServerPtr server > G_GNUC_UNUSED, > cleanup: > if (ret < 0) > virNetMessageSaveError(rerr); > -virMutexUnlock(&priv->lock); > return ret; > } > > -- > 2.25.2 I was wondering why virMutexInit() returns a value whereas g_mutex_init() does not, to make sure there weren't any additional adjustments necessary, but it's probably OK. I mean, it does feel slightly dubious as virMutexInit() fails if pthread_mutex_init() fails which can happen under a bunch of seemingly fairly realistic scenarios. I assume g_mutex_init() ultimately calls pthread_mutex_init() as well so these scenarios should still apply. However, this seems ultimately a problem of glib API designers to decide how realistic the scenarios are (at least some of them seem to be related to memory allocation which glib solves by aborting) and whether to report them to their users, and they made the decision that they made, hopefully for good reasons. Reviewed-by: Pavel Mores
Re: [PATCH 2/2] backup: Allow 'encryption' of backups and scratch images
On Tue, Apr 14, 2020 at 11:22:44AM +0200, Peter Krempa wrote: > Add the appropriate entries into the schema to allow encryption of the > backup or scratch image. Since we use blockdev internals for everything > no changes to the code are actually necessary. > > https://bugzilla.redhat.com/show_bug.cgi?id=1811906 > > Signed-off-by: Peter Krempa > --- > docs/formatbackup.html.in | 14 +++- > docs/schemas/domainbackup.rng | 65 +++ > .../backup-pull-encrypted.xml | 30 + > .../backup-push-encrypted.xml | 29 + > .../backup-pull-encrypted.xml | 30 + > .../backup-push-encrypted.xml | 29 + > tests/genericxml2xmltest.c| 3 + > 7 files changed, 185 insertions(+), 15 deletions(-) > create mode 100644 tests/domainbackupxml2xmlin/backup-pull-encrypted.xml > create mode 100644 tests/domainbackupxml2xmlin/backup-push-encrypted.xml > create mode 100644 tests/domainbackupxml2xmlout/backup-pull-encrypted.xml > create mode 100644 tests/domainbackupxml2xmlout/backup-push-encrypted.xml > > diff --git a/docs/formatbackup.html.in b/docs/formatbackup.html.in > index 87744bac98..9e69d8f7d3 100644 > --- a/docs/formatbackup.html.in > +++ b/docs/formatbackup.html.in > @@ -101,7 +101,7 @@ > block. > Similar to a disk declaration for a domain, the choice of > type > controls what additional sub-elements are needed to describe > -the destination. > +the destination. ^This hunk needs to go into the previous commit, otherwise the previous patch breaks the build. With that fixed (to both patches): Reviewed-by: Erik Skultety
Re: [PATCH 0/2] backup: Fix docs and add schema for LUKS encrypted backups
On a Tuesday in 2020, Peter Krempa wrote: Peter Krempa (2): docs: backup: Remove references to push backup to network disk backup: Allow 'encryption' of backups and scratch images docs/formatbackup.html.in | 19 -- docs/schemas/domainbackup.rng | 65 +++ .../backup-pull-encrypted.xml | 30 + .../backup-push-encrypted.xml | 29 + .../backup-pull-encrypted.xml | 30 + .../backup-push-encrypted.xml | 29 + tests/genericxml2xmltest.c| 3 + 7 files changed, 187 insertions(+), 18 deletions(-) create mode 100644 tests/domainbackupxml2xmlin/backup-pull-encrypted.xml create mode 100644 tests/domainbackupxml2xmlin/backup-push-encrypted.xml create mode 100644 tests/domainbackupxml2xmlout/backup-pull-encrypted.xml create mode 100644 tests/domainbackupxml2xmlout/backup-push-encrypted.xml Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH] README: rewrite in rst
On 4/14/20 11:11 AM, Ján Tomko wrote: Signed-off-by: Ján Tomko --- README | 2 +- README.md => README.rst | 53 + 2 files changed, 28 insertions(+), 27 deletions(-) rename README.md => README.rst (65%) Reviewed-by: Michal Privoznik Michal
[libvirt PATCH] README: Add GitLab CI badge
We've moved most of our CI jobs to GitLab, so we should display the corresponding badge prominently. Signed-off-by: Andrea Bolognani --- README.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.rst b/README.rst index 080352ac77..cdcc97b5b2 100644 --- a/README.rst +++ b/README.rst @@ -1,3 +1,6 @@ +.. image:: https://gitlab.com/libvirt/libvirt/badges/master/pipeline.svg + :target: https://gitlab.com/libvirt/libvirt/pipelines + :alt: GitLab CI Build Status .. image:: https://travis-ci.org/libvirt/libvirt.svg :target: https://travis-ci.org/libvirt/libvirt :alt: Travis CI Build Status -- 2.25.2
Re: [libvirt PATCH] Convert all remaining Markdown files to reStructuredText
On Tue, 2020-04-14 at 15:59 +0200, Ján Tomko wrote: > On a Tuesday in 2020, Andrea Bolognani wrote: > > -``` > > -$ mkdir build && cd build > > -$ ../configure --prefix=/usr --sysconfdir=/etc --localstatedir=/var > > -$ make > > -$ sudo make install > > -``` > > +:: > > Side question: > Are we against putting :: at the end of the previous line? I think it > looks slightly better that way. There are no examples of that pattern in our repository at the moment. For what it's worth, I think it looks significantly worse than having the :: on a separate line. -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt PATCH] Convert all remaining Markdown files to reStructuredText
On a Tuesday in 2020, Andrea Bolognani wrote: We've adopted reStructuredText as the primary markup language for our documentation and, given that both GitLab and GitHub can render documents in this format just fine, it makes sense to get rid of the few last remaining bits of Markdown and standardize on reStructuredText across the board. Signed-off-by: Andrea Bolognani --- Note that I've taken a few liberties during the conversion when it comes to formatting, with the goal of staying close to the style used for existing reStructuredText documents, but I have not altered the contents beyond that. ABOUT-NLS | 2 +- Makefile.am | 2 +- README| 2 +- README.md => README.rst | 55 --- docs/Makefile.am | 2 +- docs/fonts/{LICENSE.md => LICENSE.rst}| 30 + libvirt.spec.in | 2 +- po/{README.md => README.rst} | 32 - tools/wireshark/{README.md => README.rst} | 28 9 files changed, 97 insertions(+), 58 deletions(-) rename README.md => README.rst (65%) rename docs/fonts/{LICENSE.md => LICENSE.rst} (94%) rename po/{README.md => README.rst} (86%) rename tools/wireshark/{README.md => README.rst} (52%) diff --git a/README.md b/README.rst similarity index 65% rename from README.md rename to README.rst index 44b0dd87c5..663fba4510 100644 --- a/README.md +++ b/README.rst [...] @@ -1,6 +1,9 @@ -[![Build Status](https://travis-ci.org/libvirt/libvirt.svg)](https://travis-ci.org/libvirt/libvirt) - [![CII Best Practices](https://bestpractices.coreinfrastructure.org/projects/355/badge)](https://bestpractices.coreinfrastructure.org/projects/355) +.. image:: https://travis-ci.org/libvirt/libvirt.svg + :alt: Travis CI Build Status +.. image:: https://bestpractices.coreinfrastructure.org/projects/355/badge + :alt: CII Best Practices +== Libvirt API for virtualization == @@ -21,66 +24,66 @@ mappings into object systems such as GObject, CIM and SNMP. Further information about the libvirt project can be found on the website: -[https://libvirt.org](https://libvirt.org) +https://libvirt.org License +=== The libvirt C API is distributed under the terms of GNU Lesser General Public License, version 2.1 (or later). Some parts of the code that are not part of the C library may have the more restrictive GNU General -Public License, version 2.0 (or later). See the files `COPYING.LESSER` -and `COPYING` for full license terms & conditions. +Public License, version 2.0 (or later). See the files ``COPYING.LESSER`` +and ``COPYING`` for full license terms & conditions. Installation - + Libvirt uses the GNU Autotools build system, so in general can be built and installed with the usual commands, however, we mandate to have the build directory different than the source directory. For example, to build in a manner that is suitable for installing as root, use: -``` -$ mkdir build && cd build -$ ../configure --prefix=/usr --sysconfdir=/etc --localstatedir=/var -$ make -$ sudo make install -``` +:: Side question: Are we against putting :: at the end of the previous line? I think it looks slightly better that way. + + $ mkdir build && cd build + $ ../configure --prefix=/usr --sysconfdir=/etc --localstatedir=/var + $ make + $ sudo make install While to build & install as an unprivileged user -``` -$ mkdir build && cd build -$ ../configure --prefix=$HOME/usr -$ make -$ make install -``` +:: + + $ mkdir build && cd build + $ ../configure --prefix=$HOME/usr + $ make + $ make install The libvirt code relies on a large number of 3rd party libraries. These will -be detected during execution of the `configure` script and a summary printed +be detected during execution of the ``configure`` script and a summary printed which lists any missing (optional) dependencies. Contributing - + The libvirt project welcomes contributions in many ways. For most components the best way to contribute is to send patches to the primary development mailing list. Further guidance on this can be found on the website: -[https://libvirt.org/contribute.html](https://libvirt.org/contribute.html) +https://libvirt.org/contribute.html Contact +=== The libvirt project has two primary mailing lists: - * libvirt-us...@redhat.com (**for user discussions**) - * libvir-list@redhat.com (**for development only**) +* libvirt-us...@redhat.com (**for user discussions**) +* libvir-list@redhat.com (**for development only**) Further details on contacting the project are available on the website: -[https://libvirt.org/contact.html](https://libvirt.org/contact.html) +https://libvirt.org/contact.html diff --git a/docs/Makefile.am b/docs/Makefile.am index 404121a2a2..6860efc888 100644 --- a/d
Re: [libvirt PATCH] Convert all remaining Markdown files to reStructuredText
On a Tuesday in 2020, Andrea Bolognani wrote: On Tue, 2020-04-14 at 13:52 +0200, Andrea Bolognani wrote: +++ b/README.rst @@ -1,6 +1,9 @@ -[![Build Status](https://travis-ci.org/libvirt/libvirt.svg)](https://travis-ci.org/libvirt/libvirt) - [![CII Best Practices](https://bestpractices.coreinfrastructure.org/projects/355/badge)](https://bestpractices.coreinfrastructure.org/projects/355) +.. image:: https://travis-ci.org/libvirt/libvirt.svg + :alt: Travis CI Build Status +.. image:: https://bestpractices.coreinfrastructure.org/projects/355/badge + :alt: CII Best Practices The fact that the badges are clickable got lost in translation. Consider the diff below to be squashed in. diff --git a/README.rst b/README.rst index 663fba4510..080352ac77 100644 --- a/README.rst +++ b/README.rst @@ -1,6 +1,8 @@ .. image:: https://travis-ci.org/libvirt/libvirt.svg + :target: https://travis-ci.org/libvirt/libvirt :alt: Travis CI Build Status .. image:: https://bestpractices.coreinfrastructure.org/projects/355/badge + :target: https://bestpractices.coreinfrastructure.org/projects/355 :alt: CII Best Practices == Signed-off-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 2/2] conf: Don't format http cookies unless VIR_DOMAIN_DEF_FORMAT_SECURE is used
On Tue, Apr 14, 2020 at 10:38:48AM +0200, Peter Krempa wrote: > Starting from 3b076391befc3fe72deb0c244ac6c2b4c100b410 s/from/with > (v6.1.0-122-g3b076391be) we support http cookies. Since they may contain > somewhat sensitive information we should not format into the XML unless s/format/format them/ -- Erik Skultety
Re: [PATCH 0/2] Fix formatting of http cookies into XML
On Tue, Apr 14, 2020 at 10:38:46AM +0200, Peter Krempa wrote: > See 2/2. > > Peter Krempa (2): > virstoragetest: testBackingParse: Use VIR_DOMAIN_DEF_FORMAT_SECURE > when formatting xml > conf: Don't format http cookies unless VIR_DOMAIN_DEF_FORMAT_SECURE is > used > > src/conf/domain_conf.c | 8 ++-- > tests/virstoragetest.c | 3 ++- > 2 files changed, 8 insertions(+), 3 deletions(-) > > -- > 2.26.0 > Reviewed-by: Erik Skultety
Re: [libvirt PATCH] Convert all remaining Markdown files to reStructuredText
On Tue, 2020-04-14 at 13:52 +0200, Andrea Bolognani wrote: > +++ b/README.rst > @@ -1,6 +1,9 @@ > -[![Build > Status](https://travis-ci.org/libvirt/libvirt.svg)](https://travis-ci.org/libvirt/libvirt) > - [![CII Best > Practices](https://bestpractices.coreinfrastructure.org/projects/355/badge)](https://bestpractices.coreinfrastructure.org/projects/355) > +.. image:: https://travis-ci.org/libvirt/libvirt.svg > + :alt: Travis CI Build Status > +.. image:: https://bestpractices.coreinfrastructure.org/projects/355/badge > + :alt: CII Best Practices The fact that the badges are clickable got lost in translation. Consider the diff below to be squashed in. diff --git a/README.rst b/README.rst index 663fba4510..080352ac77 100644 --- a/README.rst +++ b/README.rst @@ -1,6 +1,8 @@ .. image:: https://travis-ci.org/libvirt/libvirt.svg + :target: https://travis-ci.org/libvirt/libvirt :alt: Travis CI Build Status .. image:: https://bestpractices.coreinfrastructure.org/projects/355/badge + :target: https://bestpractices.coreinfrastructure.org/projects/355 :alt: CII Best Practices == -- Andrea Bolognani / Red Hat / Virtualization
Re: libvirt python console
On 4/12/20 7:18 PM, PunkGeek wrote: Hello, I see this example about VM console serial, Which I could connect to the guest VM successfully. however, I could not understand how to send commands to the guest VM? https://github.com/libvirt/libvirt-python/blob/master/examples/consolecallback.py Best regards The example is basically what 'virsh console' does. This simply opens serial console to virtual machine. But then you need to enable agetty for the serial console. In systemd distributions it's sufficient to append 'console=ttyS0' to kernel cmd line and systemd should do the rest automagically. Michal
Re: Wiki page audit - possible page deleteion
On 4/9/20 12:10 PM, Daniel P. Berrangé wrote: On Wed, Apr 08, 2020 at 04:45:46PM +0100, Daniel P. Berrangé wrote: Various GSoC pages, the per-year pages, and for 2016 only some reports from the contributors. Most of the latter is outdated info that's no longer really relevant, so I'd probablyjust delete the last 4 pages, and move the rest to the main site. Google Summer of Code 2016 Google Summer of Code 2017 Google Summer of Code 2018 Google Summer of Code 2019 Google Summer of Code FAQ Google Summer of Code Ideas Google Summer of Code 2016/Abstracting device address allocation Google Summer of Code 2016/Asynchronous lifecycle events for storage objects Google Summer of Code 2016/Making virsh more bash like Google Summer of Code 2016/lxc migration What do people think of using the GitLab issue tracker for main Google Summer of Code ideas page ? We could have a "gsoc" label that we apply to issues to make it easy to view them all. The initial issue description would describe the high level of the project. The issue tracker can be used for further discussions as the project is planned and then gets underway, as well as letting us track progress. We could create further tags "gsoc::2018" when issues are picked for a particular year. Yes, makes sense. I'm okay with this approach. Michal
[libvirt PATCH] CONTRIBUTING: Include in release archives
The file, added with commit commit 874952f80c6d68c1a7a75e71c11a576f96f75dc2 Author: Andrea Bolognani Date: Mon Apr 6 11:56:58 2020 +0200 CONTRIBUTING: Add entry point for new contributors should be included in release archives. Signed-off-by: Andrea Bolognani --- Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile.am b/Makefile.am index 5590c88e4d..01a9a5aa83 100644 --- a/Makefile.am +++ b/Makefile.am @@ -45,6 +45,7 @@ EXTRA_DIST = \ run.in \ README.md \ AUTHORS.in \ + CONTRIBUTING.rst \ scripts/apibuild.py \ scripts/augeas-gentest.py \ build-aux/check-spacing.pl \ -- 2.25.2
[libvirt PATCH] Convert all remaining Markdown files to reStructuredText
We've adopted reStructuredText as the primary markup language for our documentation and, given that both GitLab and GitHub can render documents in this format just fine, it makes sense to get rid of the few last remaining bits of Markdown and standardize on reStructuredText across the board. Signed-off-by: Andrea Bolognani --- Note that I've taken a few liberties during the conversion when it comes to formatting, with the goal of staying close to the style used for existing reStructuredText documents, but I have not altered the contents beyond that. ABOUT-NLS | 2 +- Makefile.am | 2 +- README| 2 +- README.md => README.rst | 55 --- docs/Makefile.am | 2 +- docs/fonts/{LICENSE.md => LICENSE.rst}| 30 + libvirt.spec.in | 2 +- po/{README.md => README.rst} | 32 - tools/wireshark/{README.md => README.rst} | 28 9 files changed, 97 insertions(+), 58 deletions(-) rename README.md => README.rst (65%) rename docs/fonts/{LICENSE.md => LICENSE.rst} (94%) rename po/{README.md => README.rst} (86%) rename tools/wireshark/{README.md => README.rst} (52%) diff --git a/ABOUT-NLS b/ABOUT-NLS index b583e276a7..91a3266ed6 12 --- a/ABOUT-NLS +++ b/ABOUT-NLS @@ -1 +1 @@ -po/README.md \ No newline at end of file +po/README.rst \ No newline at end of file diff --git a/Makefile.am b/Makefile.am index 5590c88e4d..06475344c1 100644 --- a/Makefile.am +++ b/Makefile.am @@ -43,7 +43,7 @@ EXTRA_DIST = \ autogen.sh \ GNUmakefile \ run.in \ - README.md \ + README.rst \ AUTHORS.in \ scripts/apibuild.py \ scripts/augeas-gentest.py \ diff --git a/README b/README index 42061c01a1..92cacd2853 12 --- a/README +++ b/README @@ -1 +1 @@ -README.md \ No newline at end of file +README.rst \ No newline at end of file diff --git a/README.md b/README.rst similarity index 65% rename from README.md rename to README.rst index 44b0dd87c5..663fba4510 100644 --- a/README.md +++ b/README.rst @@ -1,6 +1,9 @@ -[![Build Status](https://travis-ci.org/libvirt/libvirt.svg)](https://travis-ci.org/libvirt/libvirt) - [![CII Best Practices](https://bestpractices.coreinfrastructure.org/projects/355/badge)](https://bestpractices.coreinfrastructure.org/projects/355) +.. image:: https://travis-ci.org/libvirt/libvirt.svg + :alt: Travis CI Build Status +.. image:: https://bestpractices.coreinfrastructure.org/projects/355/badge + :alt: CII Best Practices +== Libvirt API for virtualization == @@ -21,66 +24,66 @@ mappings into object systems such as GObject, CIM and SNMP. Further information about the libvirt project can be found on the website: -[https://libvirt.org](https://libvirt.org) +https://libvirt.org License +=== The libvirt C API is distributed under the terms of GNU Lesser General Public License, version 2.1 (or later). Some parts of the code that are not part of the C library may have the more restrictive GNU General -Public License, version 2.0 (or later). See the files `COPYING.LESSER` -and `COPYING` for full license terms & conditions. +Public License, version 2.0 (or later). See the files ``COPYING.LESSER`` +and ``COPYING`` for full license terms & conditions. Installation - + Libvirt uses the GNU Autotools build system, so in general can be built and installed with the usual commands, however, we mandate to have the build directory different than the source directory. For example, to build in a manner that is suitable for installing as root, use: -``` -$ mkdir build && cd build -$ ../configure --prefix=/usr --sysconfdir=/etc --localstatedir=/var -$ make -$ sudo make install -``` +:: + + $ mkdir build && cd build + $ ../configure --prefix=/usr --sysconfdir=/etc --localstatedir=/var + $ make + $ sudo make install While to build & install as an unprivileged user -``` -$ mkdir build && cd build -$ ../configure --prefix=$HOME/usr -$ make -$ make install -``` +:: + + $ mkdir build && cd build + $ ../configure --prefix=$HOME/usr + $ make + $ make install The libvirt code relies on a large number of 3rd party libraries. These will -be detected during execution of the `configure` script and a summary printed +be detected during execution of the ``configure`` script and a summary printed which lists any missing (optional) dependencies. Contributing - + The libvirt project welcomes contributions in many ways. For most components the best way to contribute is to send patches to the primary development mailing list. Further guidance on this can be found on the website: -[https://libvirt.org/contribute.html](https://libvirt.org/contribute.html) +https://libvirt.org/contribute.html Contact +=
Re: [PATCH] libvirt-stream.c: Use g_autofree
On Mon, Apr 13, 2020 at 06:19:26PM +0530, Seeteena Thoufeek wrote: > Signed-off-by: Seeteena Thoufeek > --- Reviewed-by: Erik Skultety
Re: [libvirt PATCH 1/2] CONTRIBUTING: Add entry point for new contributors
On Tue, 2020-04-14 at 10:14 +0200, Ján Tomko wrote: > On a Monday in 2020, Andrea Bolognani wrote: > > diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md > > new file mode 100644 > > index 00..a8551965bd > > --- /dev/null > > +++ b/CONTRIBUTING.md > > s/md/rst/ Good point. I've verified that both GitLab and GitHub can render reStructuredText correctly; I'll look into converting the few remaining Markdown files in the repository to reStructuredText as well. -- Andrea Bolognani / Red Hat / Virtualization
Re: [libvirt-jenkins-ci PATCH v3 4/5] guests: Introduce the new 'gitlab' flavor
On Tue, Apr 14, 2020 at 12:31:26PM +0200, Andrea Bolognani wrote: > On Tue, 2020-04-14 at 10:17 +0200, Erik Skultety wrote: > > On Thu, Apr 09, 2020 at 12:28:50PM +0200, Andrea Bolognani wrote: > > > On Thu, 2020-04-09 at 06:23 +0200, Erik Skultety wrote: > > > > +++ b/guests/playbooks/update/tasks/gitlab.yml > > > > +- name: Make {{ gitlab_runner_config_dir }} world readable > > > > + file: > > > > +path: '{{ gitlab_runner_config_dir }}' > > > > +mode: '0755' > > > > + > > > > +- name: Make {{ gitlab_runner_config_dir }}/config.toml world readable > > > > + file: > > > > +path: '{{ gitlab_runner_config_dir }}/config.toml' > > > > +mode: '0644' > > > > > > The message for these tasks is unnecessarily detailed: I'd just use > > > something like > > > > > > Make gitlab-runner configuration readable > > > > Okay, however... > > > > > for both. > > > > > > Additionally, even though the gitlab user is going to be the only one > > > on the system so it doesn't make much of a difference in practice, I > > > think we should have config.toml > > > > > > > ...here you suggest the following adjustment. I feel like the messages above > > will then become confusing and misleading, since who are we making it > > readable > > for? Well, only for the gitlab user, so I think a little more detail in > > them is > > justifiable. > > > > > owner: root > > > group: gitlab > > > mode: '0640' > > > > So how about: > > "Make gitlab-runner config dir readable" for the former and > > "Make gitlab-runner config.toml owned by the gitlab group" for the latter > > I still think that's an unnecessary amount of detail, and we have > plenty of existing examples in the repository such as > > - name: Update installed packages > package: > name: fedora-gpg-keys > state: latest > disable_gpg_check: yes > when: > - os_name == 'Fedora' > - os_version == 'Rawhide' > > - name: Update installed packages > command: '{{ package_manager }} update --refresh --exclude "kernel*" -y' > args: > warn: no > when: > - os_name == 'Fedora' > - os_version == 'Rawhide' > > - name: Update installed packages > command: '{{ package_manager }} update --disablerepo="*" > --enablerepo=fedora-rawhide-kernel-nodebug "kernel*" -y' > args: > warn: no > when: > - os_name == 'Fedora' > - os_version == 'Rawhide' Fair enough, ^this is a compelling counter-argument, I proceeded with the original suggestion and merged the series, thanks for the review. -- Erik Skultety
Re: [libvirt PATCH 2/3] travis: Reduce test matrix
On Tue, 2020-04-14 at 10:54 +0100, Daniel P. Berrangé wrote: > On Fri, Apr 10, 2020 at 11:10:33AM +0200, Andrea Bolognani wrote: > > The xcode10.3 build has recently started failing because of issues > > that are not in libvirt, so get rid of it. It didn't buy us > > additionaly platform coverage anyway, since underneath it's using > > the same macOS 10.14 version as xcode11.3. > > The point was to test both xcode versions, since they're both commonly > used. I was testing this over the weekend and determined that 10.1 > will still work correctly. Travis broke something in their 10.2/10.3 > images that inserts a bogus -I path. Considering that almost nobody in the libvirt community is actually focused on macOS support, trying to chase after two SDKs instead of a single one doesn't necessarily sound like a good idea to me, especially when the older one doesn't target the current version of macOS. It's already pretty bad that we're stuck on 10.14, as opposed to 10.15 which is the version we actually target according to our platform support policy, because Travis CI always takes forever to prepare new images. I was looking into Cirrus CI the other day and they support macOS too, and they even already have a 10.15 image. I didn't dig deeper, but maybe if we're stuck using an external service for macOS builds it should be Cirrus CI instead of Travis CI. -- Andrea Bolognani / Red Hat / Virtualization
Re: [PATCH ocaml 1/1] Add dune build system
On Tue, Apr 14, 2020 at 12:22:49PM +0200, Pino Toscano wrote: > Introduce the dune build system to build ocaml-libvirt, providing > everything that the current autoconf-based build system has. > > Add also an opam configuration files, as it is mandatory with dune. > > Signed-off-by: Pino Toscano ACK You might want to drop -safe-string since it's been the default for a while (since OCaml 4.06 / Nov 2017). Rich. > .gitignore | 3 +++ > dune | 18 + > dune-project | 1 + > dune.inc | 22 > examples/dune | 50 > libvirt/discover.ml| 40 + > libvirt/dune | 57 ++ > libvirt/dune-project | 2 ++ > libvirt/mllibvirt.opam | 33 > 9 files changed, 226 insertions(+) > create mode 100644 dune > create mode 100644 dune-project > create mode 100644 dune.inc > create mode 100644 examples/dune > create mode 100644 libvirt/discover.ml > create mode 100644 libvirt/dune > create mode 100644 libvirt/dune-project > create mode 100644 libvirt/mllibvirt.opam > > diff --git a/.gitignore b/.gitignore > index c52c1b8..814457b 100644 > --- a/.gitignore > +++ b/.gitignore > @@ -10,10 +10,12 @@ > *.opt > *.so > *~ > +.merlin > Make.rules > Makefile > core > core.* > +/_build/ > /META > /aclocal.m4 > /autom4te.cache > @@ -30,5 +32,6 @@ core.* > /html/ > /libvirt/libvirt_generated.c > /libvirt/libvirt_version.ml > +/libvirt/mllibvirt.install > /ocaml-libvirt-*.exe > /ocaml-libvirt-*.tar.gz > diff --git a/dune b/dune > new file mode 100644 > index 000..5002345 > --- /dev/null > +++ b/dune > @@ -0,0 +1,18 @@ > +; OCaml bindings for libvirt. > +; (C) Copyright 2020 Pino Toscano, Red Hat Inc. > +; > +; This library is free software; you can redistribute it and/or > +; modify it under the terms of the GNU Lesser General Public > +; License as published by the Free Software Foundation; either > +; version 2 of the License, or (at your option) any later version. > +; > +; This library is distributed in the hope that it will be useful, > +; but WITHOUT ANY WARRANTY; without even the implied warranty of > +; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > +; Lesser General Public License for more details. > +; > +; You should have received a copy of the GNU Lesser General Public > +; License along with this library; if not, write to the Free Software > +; Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 > USA > + > +(include dune.inc) > diff --git a/dune-project b/dune-project > new file mode 100644 > index 000..de4fc20 > --- /dev/null > +++ b/dune-project > @@ -0,0 +1 @@ > +(lang dune 1.0) > diff --git a/dune.inc b/dune.inc > new file mode 100644 > index 000..2cfb767 > --- /dev/null > +++ b/dune.inc > @@ -0,0 +1,22 @@ > +; OCaml bindings for libvirt. > +; (C) Copyright 2020 Pino Toscano, Red Hat Inc. > +; > +; This library is free software; you can redistribute it and/or > +; modify it under the terms of the GNU Lesser General Public > +; License as published by the Free Software Foundation; either > +; version 2 of the License, or (at your option) any later version. > +; > +; This library is distributed in the hope that it will be useful, > +; but WITHOUT ANY WARRANTY; without even the implied warranty of > +; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > +; Lesser General Public License for more details. > +; > +; You should have received a copy of the GNU Lesser General Public > +; License along with this library; if not, write to the Free Software > +; Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 > USA > + > +(env > + (dev > +(flags (:standard -g -warn-error CDEFLMPSUVYZX-3 -w -9-27-34-37-50 > -safe-string))) > + (release > +(flags (:standard -g -warn-error CDEFLMPSUVYZX-3 -w -9-27-34-37-50 > -safe-string > diff --git a/examples/dune b/examples/dune > new file mode 100644 > index 000..219727c > --- /dev/null > +++ b/examples/dune > @@ -0,0 +1,50 @@ > +; OCaml bindings for libvirt. > +; (C) Copyright 2020 Pino Toscano, Red Hat Inc. > +; > +; This library is free software; you can redistribute it and/or > +; modify it under the terms of the GNU Lesser General Public > +; License as published by the Free Software Foundation; either > +; version 2 of the License, or (at your option) any later version. > +; > +; This library is distributed in the hope that it will be useful, > +; but WITHOUT ANY WARRANTY; without even the implied warranty of > +; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > +; Lesser General Public License for more details. > +; > +; You should have received a copy of the GNU Lesser General Public > +; License along with this library; if not, write to the Free Software > +; Foundation, Inc., 51 Franklin Street,
Re: [libvirt-jenkins-ci PATCH v3 4/5] guests: Introduce the new 'gitlab' flavor
On Tue, 2020-04-14 at 10:17 +0200, Erik Skultety wrote: > On Thu, Apr 09, 2020 at 12:28:50PM +0200, Andrea Bolognani wrote: > > On Thu, 2020-04-09 at 06:23 +0200, Erik Skultety wrote: > > > +++ b/guests/playbooks/update/tasks/gitlab.yml > > > +- name: Make {{ gitlab_runner_config_dir }} world readable > > > + file: > > > +path: '{{ gitlab_runner_config_dir }}' > > > +mode: '0755' > > > + > > > +- name: Make {{ gitlab_runner_config_dir }}/config.toml world readable > > > + file: > > > +path: '{{ gitlab_runner_config_dir }}/config.toml' > > > +mode: '0644' > > > > The message for these tasks is unnecessarily detailed: I'd just use > > something like > > > > Make gitlab-runner configuration readable > > Okay, however... > > > for both. > > > > Additionally, even though the gitlab user is going to be the only one > > on the system so it doesn't make much of a difference in practice, I > > think we should have config.toml > > > > ...here you suggest the following adjustment. I feel like the messages above > will then become confusing and misleading, since who are we making it readable > for? Well, only for the gitlab user, so I think a little more detail in them > is > justifiable. > > > owner: root > > group: gitlab > > mode: '0640' > > So how about: > "Make gitlab-runner config dir readable" for the former and > "Make gitlab-runner config.toml owned by the gitlab group" for the latter I still think that's an unnecessary amount of detail, and we have plenty of existing examples in the repository such as - name: Update installed packages package: name: fedora-gpg-keys state: latest disable_gpg_check: yes when: - os_name == 'Fedora' - os_version == 'Rawhide' - name: Update installed packages command: '{{ package_manager }} update --refresh --exclude "kernel*" -y' args: warn: no when: - os_name == 'Fedora' - os_version == 'Rawhide' - name: Update installed packages command: '{{ package_manager }} update --disablerepo="*" --enablerepo=fedora-rawhide-kernel-nodebug "kernel*" -y' args: warn: no when: - os_name == 'Fedora' - os_version == 'Rawhide' where we provide the high-level information as feedback to the user, without going too much into detail - in this case, that we're updating the system in three steps instead of a single one because some packages require special handling. But I don't want to hold up the series because of bikeshedding, so if you are very keen on having the extra detail I'll take it as-is :) -- Andrea Bolognani / Red Hat / Virtualization
[PATCH ocaml 0/1] RFC: add/switch to dune build system
ocaml-libvirt is currently built using manually written makefiles, and an autoconf script to detect libvirt (using pkg-config). While this seems OK, it has a lot of downsides: - all the OCaml rules are manually written - there is some form of duplication in the OCaml rules - the bytecode vs native build is fragile Initially named jhbuild, dune is a native OCaml build system which plans to eventually replace the others (omake, oasis, ocamlbuild), with a growing user base in the OCaml community. This is a very simple patch adding support for dune. The idea is to replace the current autoconf+makefile build system, which can be removed more or less easily. I'd prefer to not maintain two build systems at once, so this patch is also a way to start a discussion about the proposed approach. Things not yet done (pending the discussion): - rewrite/simplify the release bits, currently interwonen in the makefiles - update the documentation - update libvirt-ci - remove the autoconf+makefile build system Pino Toscano (1): Add dune build system .gitignore | 3 +++ dune | 18 + dune-project | 1 + dune.inc | 22 examples/dune | 50 libvirt/discover.ml| 40 + libvirt/dune | 57 ++ libvirt/dune-project | 2 ++ libvirt/mllibvirt.opam | 33 9 files changed, 226 insertions(+) create mode 100644 dune create mode 100644 dune-project create mode 100644 dune.inc create mode 100644 examples/dune create mode 100644 libvirt/discover.ml create mode 100644 libvirt/dune create mode 100644 libvirt/dune-project create mode 100644 libvirt/mllibvirt.opam -- 2.25.2
[PATCH ocaml 1/1] Add dune build system
Introduce the dune build system to build ocaml-libvirt, providing everything that the current autoconf-based build system has. Add also an opam configuration files, as it is mandatory with dune. Signed-off-by: Pino Toscano --- .gitignore | 3 +++ dune | 18 + dune-project | 1 + dune.inc | 22 examples/dune | 50 libvirt/discover.ml| 40 + libvirt/dune | 57 ++ libvirt/dune-project | 2 ++ libvirt/mllibvirt.opam | 33 9 files changed, 226 insertions(+) create mode 100644 dune create mode 100644 dune-project create mode 100644 dune.inc create mode 100644 examples/dune create mode 100644 libvirt/discover.ml create mode 100644 libvirt/dune create mode 100644 libvirt/dune-project create mode 100644 libvirt/mllibvirt.opam diff --git a/.gitignore b/.gitignore index c52c1b8..814457b 100644 --- a/.gitignore +++ b/.gitignore @@ -10,10 +10,12 @@ *.opt *.so *~ +.merlin Make.rules Makefile core core.* +/_build/ /META /aclocal.m4 /autom4te.cache @@ -30,5 +32,6 @@ core.* /html/ /libvirt/libvirt_generated.c /libvirt/libvirt_version.ml +/libvirt/mllibvirt.install /ocaml-libvirt-*.exe /ocaml-libvirt-*.tar.gz diff --git a/dune b/dune new file mode 100644 index 000..5002345 --- /dev/null +++ b/dune @@ -0,0 +1,18 @@ +; OCaml bindings for libvirt. +; (C) Copyright 2020 Pino Toscano, Red Hat Inc. +; +; This library is free software; you can redistribute it and/or +; modify it under the terms of the GNU Lesser General Public +; License as published by the Free Software Foundation; either +; version 2 of the License, or (at your option) any later version. +; +; This library is distributed in the hope that it will be useful, +; but WITHOUT ANY WARRANTY; without even the implied warranty of +; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +; Lesser General Public License for more details. +; +; You should have received a copy of the GNU Lesser General Public +; License along with this library; if not, write to the Free Software +; Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +(include dune.inc) diff --git a/dune-project b/dune-project new file mode 100644 index 000..de4fc20 --- /dev/null +++ b/dune-project @@ -0,0 +1 @@ +(lang dune 1.0) diff --git a/dune.inc b/dune.inc new file mode 100644 index 000..2cfb767 --- /dev/null +++ b/dune.inc @@ -0,0 +1,22 @@ +; OCaml bindings for libvirt. +; (C) Copyright 2020 Pino Toscano, Red Hat Inc. +; +; This library is free software; you can redistribute it and/or +; modify it under the terms of the GNU Lesser General Public +; License as published by the Free Software Foundation; either +; version 2 of the License, or (at your option) any later version. +; +; This library is distributed in the hope that it will be useful, +; but WITHOUT ANY WARRANTY; without even the implied warranty of +; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +; Lesser General Public License for more details. +; +; You should have received a copy of the GNU Lesser General Public +; License along with this library; if not, write to the Free Software +; Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +(env + (dev +(flags (:standard -g -warn-error CDEFLMPSUVYZX-3 -w -9-27-34-37-50 -safe-string))) + (release +(flags (:standard -g -warn-error CDEFLMPSUVYZX-3 -w -9-27-34-37-50 -safe-string diff --git a/examples/dune b/examples/dune new file mode 100644 index 000..219727c --- /dev/null +++ b/examples/dune @@ -0,0 +1,50 @@ +; OCaml bindings for libvirt. +; (C) Copyright 2020 Pino Toscano, Red Hat Inc. +; +; This library is free software; you can redistribute it and/or +; modify it under the terms of the GNU Lesser General Public +; License as published by the Free Software Foundation; either +; version 2 of the License, or (at your option) any later version. +; +; This library is distributed in the hope that it will be useful, +; but WITHOUT ANY WARRANTY; without even the implied warranty of +; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +; Lesser General Public License for more details. +; +; You should have received a copy of the GNU Lesser General Public +; License along with this library; if not, write to the Free Software +; Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +(executable + (name domain_events) + (modules Domain_events) + (libraries mllibvirt)) + +(executable + (name get_all_domain_stats) + (modules Get_all_domain_stats) + (libraries mllibvirt unix)) + +(executable + (name get_cpu_stats) + (modules Get_cpu_stats) + (libraries mllibvirt)) + +(executable + (name list_domains) + (modules List_domains) + (libraries mllibvirt)) + +(executable + (name
Re: [libvirt PATCH 2/3] travis: Reduce test matrix
On Tue, Apr 14, 2020 at 10:54:16AM +0100, Daniel P. Berrangé wrote: > On Fri, Apr 10, 2020 at 11:10:33AM +0200, Andrea Bolognani wrote: > > The xcode10.3 build has recently started failing because of issues > > that are not in libvirt, so get rid of it. It didn't buy us > > additionaly platform coverage anyway, since underneath it's using > > the same macOS 10.14 version as xcode11.3. > > The point was to test both xcode versions, since they're both commonly > used. I was testing this over the weekend and determined that 10.1 > will still work correctly. Travis broke something in their 10.2/10.3 > images that inserts a bogus -I path. It appears they realized their mistake, as the latest build this morning on 10.3 has succeeeded again. So we don't need to drop / change anything. 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 :|
Re: what a correct use for virConnectDomainEventRegisterAny API, how to Obtain a stable expected result
On Mon, Apr 13, 2020 at 05:32:38PM +0800, thomas.kuang wrote: > HI, everyone: > > > My target deal with network hotplug use virDomainDetachDeviceFlags. Because > when the API return ,the network maybe doesn’t remove from my vm guest os. > > So I use virConnectDomainEventRegisterAny to register an event ID: > VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED , > > my process as follow: > > cb_para->call_id=virConnectDomainEventRegisterAny(cb_para->conn,cb_para->dom,VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED, > VIR_DOMAIN_EVENT_CALLBACK(vnf_control_del_network_cb), cb_para, > vnf_control_del_network_cb_free); > > > > flags |= VIR_DOMAIN_AFFECT_CONFIG; > if (virDomainIsActive(dom) == 1) { > flags |= VIR_DOMAIN_AFFECT_LIVE; > } > > ret = virDomainDetachDeviceFlags(dom, xml, flags); > > > > > above code write in thread loop ,then in the same loop : > > > >while (1) { > > mission = vnf_mission_queue_get(task); > > if (mission == NULL) { > > sleep(1); > > continue; > > } > > vnf_op_process(&mission->info); // this will deal with network > hotplug,will call virConnectDomainEventRegisterAny then call > virDomainDetachDeviceFlags > > if (mission) { > > vnf_mission_free(mission); > > } > > if(virEventRunDefaultImpl() < 0) { // at here process the > registered callback for event-registered > > printf(); > > > } > >} > > My problem is: some time , the virEventRunDefaultImpl can trigger the > vnf_control_del_network_cb callback ,but some time there is nothing, > as if the VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED has lost. > what cause the Unpredictable behavior ? what is a correct use for > virConnectDomainEventRegisterAny ? The virDomainDetachDeviceFlags API is asynchronous and requires a cooperative guest to complete. It merely injects an ACPI PCI unplug request to the guest OS. If the guest OS doesn't honour this request (because it is in early bootup, or is in BIOS, or is crashed, or is configured to disable hotplug, or is maliciously not responding), then the device will never be unplugged, and so you'll never get an event. So the first thing for you todo is to validate that the device really is being unplugged in the guest OS, and in QEMU. To check QEMU run: virsh qemu-monitor-command --hmp $GUEST "info pci" and look to see if the PCI device is still listed in the output. 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 :|
libvirt python console
Hello, I see this example about VM console serial, Which I could connect to the guest VM successfully. however, I could not understand how to send commands to the guest VM? https://github.com/libvirt/libvirt-python/blob/master/examples/consolecallback.py Best regards
Re: [libvirt PATCH 3/3] travis: Remove usage of 'sudo'
On Fri, Apr 10, 2020 at 11:10:34AM +0200, Andrea Bolognani wrote: > Travis CI reports > > root: deprecated key sudo (The key `sudo` has no effect anymore.) > > so let's drop it. > > Signed-off-by: Andrea Bolognani > --- > .travis.yml | 1 - > 1 file changed, 1 deletion(-) Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH 1/3] travis: Deduplicate build instructions
On Fri, Apr 10, 2020 at 11:10:32AM +0200, Andrea Bolognani wrote: > All information, except for osx_image image, is identical between > the two jobs so we can avoid repeating it. > > Signed-off-by: Andrea Bolognani > --- > .travis.yml | 51 --- > 1 file changed, 20 insertions(+), 31 deletions(-) Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [libvirt PATCH 2/3] travis: Reduce test matrix
On Fri, Apr 10, 2020 at 11:10:33AM +0200, Andrea Bolognani wrote: > The xcode10.3 build has recently started failing because of issues > that are not in libvirt, so get rid of it. It didn't buy us > additionaly platform coverage anyway, since underneath it's using > the same macOS 10.14 version as xcode11.3. The point was to test both xcode versions, since they're both commonly used. I was testing this over the weekend and determined that 10.1 will still work correctly. Travis broke something in their 10.2/10.3 images that inserts a bogus -I path. 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 :|
Re: [PATCH] driver.c: Use g_autofree
On Mon, Apr 13, 2020 at 06:18:57PM +0530, Seeteena Thoufeek wrote: > This is the only instance of g_autofree change applicable for > driver.c I'd say something like: "This is the last missing g_autofree change to convert the module after commit 1e2ae2e311c took care of the VIR_AUTOFREE conversion". Reviewed-by: Erik Skultety I'll merge this with the adjusted commit message.
[PATCH] qemu: Revoke access to mirror on failed blockcopy
When preparing to do a blockcopy, the mirror image is modified so that QEMU can access it. For instance, the mirror has seclabels set, if it is a NVMe disk it is detached from the host and so on. And usually, the restore is done upon successful finish of the blockcopy operation. But, if something fails then we need to explicitly revoke the access to the mirror image (and thus reattach NVMe disk back to the host). Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1822538 Signed-off-by: Michal Privoznik --- src/qemu/qemu_driver.c | 4 1 file changed, 4 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 31f199fdef..9475235f01 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -17950,6 +17950,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, virDomainDiskDefPtr disk = NULL; int ret = -1; bool need_unlink = false; +bool need_revoke = false; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); const char *format = NULL; bool mirror_reuse = !!(flags & VIR_DOMAIN_BLOCK_COPY_REUSE_EXT); @@ -18124,6 +18125,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, if (qemuDomainStorageSourceChainAccessAllow(driver, vm, mirror) < 0) goto endjob; +need_revoke = true; if (blockdev) { if (mirror_reuse) { @@ -18240,6 +18242,8 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, if (crdata) qemuBlockStorageSourceAttachRollback(priv->mon, crdata->srcdata[0]); ignore_value(qemuDomainObjExitMonitor(driver, vm)); +if (need_revoke) +qemuDomainStorageSourceChainAccessRevoke(driver, vm, mirror); } if (need_unlink && virStorageFileUnlink(mirror) < 0) VIR_WARN("%s", _("unable to remove just-created copy target")); -- 2.24.1
[PATCH 0/2] backup: Fix docs and add schema for LUKS encrypted backups
Peter Krempa (2): docs: backup: Remove references to push backup to network disk backup: Allow 'encryption' of backups and scratch images docs/formatbackup.html.in | 19 -- docs/schemas/domainbackup.rng | 65 +++ .../backup-pull-encrypted.xml | 30 + .../backup-push-encrypted.xml | 29 + .../backup-pull-encrypted.xml | 30 + .../backup-push-encrypted.xml | 29 + tests/genericxml2xmltest.c| 3 + 7 files changed, 187 insertions(+), 18 deletions(-) create mode 100644 tests/domainbackupxml2xmlin/backup-pull-encrypted.xml create mode 100644 tests/domainbackupxml2xmlin/backup-push-encrypted.xml create mode 100644 tests/domainbackupxml2xmlout/backup-pull-encrypted.xml create mode 100644 tests/domainbackupxml2xmlout/backup-push-encrypted.xml -- 2.26.0
[PATCH 1/2] docs: backup: Remove references to push backup to network disk
It was never implemented and for now I don't think there's demand to do it. Remove the reference. https://bugzilla.redhat.com/show_bug.cgi?id=1812100 Signed-off-by: Peter Krempa --- docs/formatbackup.html.in | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/formatbackup.html.in b/docs/formatbackup.html.in index 55acd13ddc..87744bac98 100644 --- a/docs/formatbackup.html.in +++ b/docs/formatbackup.html.in @@ -97,12 +97,11 @@ type A mandatory attribute to describe the type of the disk, except when backup='no' is -used. Valid values include file, -block, or network. +used. Valid values include file, or +block. Similar to a disk declaration for a domain, the choice of type controls what additional sub-elements are needed to describe -the destination (such as protocol for a -network destination). +the destination. target Valid only for push mode backups, this is the primary sub-element that describes the file name of -- 2.26.0
[PATCH 2/2] backup: Allow 'encryption' of backups and scratch images
Add the appropriate entries into the schema to allow encryption of the backup or scratch image. Since we use blockdev internals for everything no changes to the code are actually necessary. https://bugzilla.redhat.com/show_bug.cgi?id=1811906 Signed-off-by: Peter Krempa --- docs/formatbackup.html.in | 14 +++- docs/schemas/domainbackup.rng | 65 +++ .../backup-pull-encrypted.xml | 30 + .../backup-push-encrypted.xml | 29 + .../backup-pull-encrypted.xml | 30 + .../backup-push-encrypted.xml | 29 + tests/genericxml2xmltest.c| 3 + 7 files changed, 185 insertions(+), 15 deletions(-) create mode 100644 tests/domainbackupxml2xmlin/backup-pull-encrypted.xml create mode 100644 tests/domainbackupxml2xmlin/backup-push-encrypted.xml create mode 100644 tests/domainbackupxml2xmlout/backup-pull-encrypted.xml create mode 100644 tests/domainbackupxml2xmlout/backup-push-encrypted.xml diff --git a/docs/formatbackup.html.in b/docs/formatbackup.html.in index 87744bac98..9e69d8f7d3 100644 --- a/docs/formatbackup.html.in +++ b/docs/formatbackup.html.in @@ -101,7 +101,7 @@ block. Similar to a disk declaration for a domain, the choice of type controls what additional sub-elements are needed to describe -the destination. +the destination. target Valid only for push mode backups, this is the primary sub-element that describes the file name of @@ -110,7 +110,8 @@ disk. An optional sub-element driver can also be used, with an attribute type to specify a destination format different from -qcow2. +qcow2. See documentation for scratch below for +additional configuration. scratch Valid only for pull mode backups, this is the primary sub-element that describes the file name of @@ -130,7 +131,14 @@ used without modification. The file is not deleted after the backup but the contents of the file don't make sense outside of the backup. The same applies for the block device which -must be formatted appropriately. +must be formatted appropriately. + +Similarly to the domain +disk +definition scratch and target can +contain seclabel and/or encryption +subelements to configure the corresponding properties. + diff --git a/docs/schemas/domainbackup.rng b/docs/schemas/domainbackup.rng index 395ea841f9..ac5b12c463 100644 --- a/docs/schemas/domainbackup.rng +++ b/docs/schemas/domainbackup.rng @@ -7,6 +7,27 @@ + + + + + luks + + + + + + + + + + + + + + + + @@ -123,9 +144,14 @@ - - - + + + + + + + + @@ -142,9 +168,14 @@ - - - + + + + + + + + @@ -192,9 +223,14 @@ - - - + + + + + + + + @@ -210,9 +246,14 @@ - - - + + + + + + + + diff --git a/tests/domainbackupxml2xmlin/backup-pull-encrypted.xml b/tests/do
[libvirt PATCH] README: rewrite in rst
Signed-off-by: Ján Tomko --- README | 2 +- README.md => README.rst | 53 + 2 files changed, 28 insertions(+), 27 deletions(-) rename README.md => README.rst (65%) diff --git a/README b/README index 42061c01a1..92cacd2853 12 --- a/README +++ b/README @@ -1 +1 @@ -README.md \ No newline at end of file +README.rst \ No newline at end of file diff --git a/README.md b/README.rst similarity index 65% rename from README.md rename to README.rst index 44b0dd87c5..222ba55e78 100644 --- a/README.md +++ b/README.rst @@ -1,6 +1,11 @@ -[![Build Status](https://travis-ci.org/libvirt/libvirt.svg)](https://travis-ci.org/libvirt/libvirt) - [![CII Best Practices](https://bestpractices.coreinfrastructure.org/projects/355/badge)](https://bestpractices.coreinfrastructure.org/projects/355) +.. image:: https://travis-ci.org/libvirt/libvirt.svg + :alt: Build Status + :target: https://travis-ci.org/libvirt/libvirt +.. image:: https://bestpractices.coreinfrastructure.org/projects/355/badge + :alt: CII Best Practices + :target: https://bestpractices.coreinfrastructure.org/projects/355 +== Libvirt API for virtualization == @@ -21,60 +26,56 @@ mappings into object systems such as GObject, CIM and SNMP. Further information about the libvirt project can be found on the website: -[https://libvirt.org](https://libvirt.org) +https://libvirt.org License +=== The libvirt C API is distributed under the terms of GNU Lesser General Public License, version 2.1 (or later). Some parts of the code that are not part of the C library may have the more restrictive GNU General -Public License, version 2.0 (or later). See the files `COPYING.LESSER` -and `COPYING` for full license terms & conditions. +Public License, version 2.0 (or later). See the files ``COPYING.LESSER`` +and ``COPYING`` for full license terms & conditions. Installation - + Libvirt uses the GNU Autotools build system, so in general can be built and installed with the usual commands, however, we mandate to have the build directory different than the source directory. For example, to build -in a manner that is suitable for installing as root, use: +in a manner that is suitable for installing as root, use:: -``` -$ mkdir build && cd build -$ ../configure --prefix=/usr --sysconfdir=/etc --localstatedir=/var -$ make -$ sudo make install -``` + $ mkdir build && cd build + $ ../configure --prefix=/usr --sysconfdir=/etc --localstatedir=/var + $ make + $ sudo make install -While to build & install as an unprivileged user +While to build & install as an unprivileged user:: -``` -$ mkdir build && cd build -$ ../configure --prefix=$HOME/usr -$ make -$ make install -``` + $ mkdir build && cd build + $ ../configure --prefix=$HOME/usr + $ make + $ make install The libvirt code relies on a large number of 3rd party libraries. These will -be detected during execution of the `configure` script and a summary printed +be detected during execution of the ``configure`` script and a summary printed which lists any missing (optional) dependencies. Contributing - + The libvirt project welcomes contributions in many ways. For most components the best way to contribute is to send patches to the primary development mailing list. Further guidance on this can be found on the website: -[https://libvirt.org/contribute.html](https://libvirt.org/contribute.html) +https://libvirt.org/contribute.html Contact +=== The libvirt project has two primary mailing lists: @@ -83,4 +84,4 @@ The libvirt project has two primary mailing lists: Further details on contacting the project are available on the website: -[https://libvirt.org/contact.html](https://libvirt.org/contact.html) +https://libvirt.org/contact.html -- 2.25.1
[PATCH 0/2] Fix formatting of http cookies into XML
See 2/2. Peter Krempa (2): virstoragetest: testBackingParse: Use VIR_DOMAIN_DEF_FORMAT_SECURE when formatting xml conf: Don't format http cookies unless VIR_DOMAIN_DEF_FORMAT_SECURE is used src/conf/domain_conf.c | 8 ++-- tests/virstoragetest.c | 3 ++- 2 files changed, 8 insertions(+), 3 deletions(-) -- 2.26.0
[PATCH 1/2] virstoragetest: testBackingParse: Use VIR_DOMAIN_DEF_FORMAT_SECURE when formatting xml
We want to format even the secure information in tests. Signed-off-by: Peter Krempa --- tests/virstoragetest.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 6e8ebeba13..6d2b21c25f 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -594,6 +594,7 @@ testBackingParse(const void *args) g_autoptr(virStorageSource) src = NULL; int rc; int erc = data->rv; +unsigned int xmlformatflags = VIR_DOMAIN_DEF_FORMAT_SECURE; /* expect failure return code with NULL expected data */ if (!data->expect) @@ -613,7 +614,7 @@ testBackingParse(const void *args) return -1; } -if (virDomainDiskSourceFormat(&buf, src, "source", 0, false, 0, true, NULL) < 0 || +if (virDomainDiskSourceFormat(&buf, src, "source", 0, false, xmlformatflags, true, NULL) < 0 || !(xml = virBufferContentAndReset(&buf))) { fprintf(stderr, "failed to format disk source xml\n"); return -1; -- 2.26.0
[PATCH 2/2] conf: Don't format http cookies unless VIR_DOMAIN_DEF_FORMAT_SECURE is used
Starting from 3b076391befc3fe72deb0c244ac6c2b4c100b410 (v6.1.0-122-g3b076391be) we support http cookies. Since they may contain somewhat sensitive information we should not format into the XML unless VIR_DOMAIN_DEF_FORMAT_SECURE is asserted. Reported-by: Han Han Signed-off-by: Peter Krempa --- src/conf/domain_conf.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8e8146374c..8700d56761 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -24636,11 +24636,15 @@ virDomainSourceDefFormatSeclabel(virBufferPtr buf, static void virDomainDiskSourceFormatNetworkCookies(virBufferPtr buf, -virStorageSourcePtr src) +virStorageSourcePtr src, +unsigned int flags) { g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); size_t i; +if (!(flags & VIR_DOMAIN_DEF_FORMAT_SECURE)) +return; + for (i = 0; i < src->ncookies; i++) { virBufferEscapeString(&childBuf, "", src->cookies[i]->name); virBufferEscapeString(&childBuf, "%s\n", src->cookies[i]->value); @@ -24701,7 +24705,7 @@ virDomainDiskSourceFormatNetwork(virBufferPtr attrBuf, virTristateBoolTypeToString(src->sslverify)); } -virDomainDiskSourceFormatNetworkCookies(childBuf, src); +virDomainDiskSourceFormatNetworkCookies(childBuf, src, flags); if (src->readahead) virBufferAsprintf(childBuf, "\n", src->readahead); -- 2.26.0
Re: [libvirt-jenkins-ci PATCH v3 4/5] guests: Introduce the new 'gitlab' flavor
On Thu, Apr 09, 2020 at 12:28:50PM +0200, Andrea Bolognani wrote: > On Thu, 2020-04-09 at 06:23 +0200, Erik Skultety wrote: > > +++ b/guests/playbooks/update/tasks/gitlab.yml > > +- name: Make {{ gitlab_runner_config_dir }} world readable > > + file: > > +path: '{{ gitlab_runner_config_dir }}' > > +mode: '0755' > > + > > +- name: Make {{ gitlab_runner_config_dir }}/config.toml world readable > > + file: > > +path: '{{ gitlab_runner_config_dir }}/config.toml' > > +mode: '0644' > > The message for these tasks is unnecessarily detailed: I'd just use > something like > > Make gitlab-runner configuration readable Okay, however... > > for both. > > Additionally, even though the gitlab user is going to be the only one > on the system so it doesn't make much of a difference in practice, I > think we should have config.toml > ...here you suggest the following adjustment. I feel like the messages above will then become confusing and misleading, since who are we making it readable for? Well, only for the gitlab user, so I think a little more detail in them is justifiable. > owner: root > group: gitlab > mode: '0640' So how about: "Make gitlab-runner config dir readable" for the former and "Make gitlab-runner config.toml owned by the gitlab group" for the latter -- Erik Skultety
Re: [libvirt PATCH 2/2] README-hacking: Drop from the git repository
On a Monday in 2020, Andrea Bolognani wrote: The newly-introduced CONTRIBUTING.md serves the same purposes and lives in the path where most people would look for it. Signed-off-by: Andrea Bolognani --- README-hacking | 56 -- 1 file changed, 56 deletions(-) delete mode 100644 README-hacking Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [libvirt PATCH 1/2] CONTRIBUTING: Add entry point for new contributors
On a Monday in 2020, Andrea Bolognani wrote: It's generally expected that a git repository will contain this file, which serves as an entry point for people interested in contributing to the project. In our case, we have extensive documentation available on the website which we don't want to duplicate, so let's just point people there. Signed-off-by: Andrea Bolognani --- CONTRIBUTING.md | 18 ++ 1 file changed, 18 insertions(+) create mode 100644 CONTRIBUTING.md diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 00..a8551965bd --- /dev/null +++ b/CONTRIBUTING.md s/md/rst/ @@ -0,0 +1,18 @@ +Contributing to libvirt +=== + +Full, up to date information on how to contribute to libvirt can be +found on the libvirt website: + +[https://libvirt.org/hacking.html](https://libvirt.org/hacking.html) https://libvirt.org/contribute.html + +To build the same document locally, from the top level directory of +your git clone run: + +``` +$ mkdir build && cd build +$ ../autogen.sh +$ make +``` + +You'll find the freshly-built document in `docs/hacking.html`. s/hacking/contribute/ -- Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature
Re: [PATCH 05/43] esx: convert virMutex to GMutex
Pino, thank you for the review. Could you please take a look at patch #42 in this series? It's the one in which I had to add some explicit unlock calls, so it'd be good for someone who knows the code to review this part. On Tue, Apr 14, 2020 at 8:15 AM Pino Toscano wrote: > > On Friday, 10 April 2020 15:54:32 CEST Rafael Fonseca wrote: > > @@ -346,11 +342,12 @@ esxStreamClose(virStreamPtr stream, bool finish) > > { > > int result = 0; > > esxStreamPrivate *priv = stream->privateData; > > +g_autoptr(GMutexLocker) locker = NULL; > > > > if (!priv) > > return 0; > > > > -virMutexLock(&priv->curl->lock); > > +locker = g_mutex_locker_new(&priv->curl->lock); > > > > if (finish && priv->backlog_used > 0) { > > virReportError(VIR_ERR_INTERNAL_ERROR, "%s", > > @@ -360,8 +357,6 @@ esxStreamClose(virStreamPtr stream, bool finish) > > > > stream->privateData = NULL; > > > > -virMutexUnlock(&priv->curl->lock); > > - > > esxFreeStreamPrivate(&priv); > > Careful here, this is a problematic situation: > - the lock is indirectly part of the @priv structure > - esxFreeStreamPrivate calls esxVI_CURL_Free(priv->curl) > - esxVI_CURL_Free calls virMutexDestroy(&item->lock) > - lock is still locked, so it will deadlock (or crash, or something > not good anyway) > > You must unlock the mutex before esxFreeStreamPrivate is called. Ops, nice catch. > I did not check other patches of this long series for similar potential > issues, please do check them. Ok, will do. > > diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c > > index 16690edfbe..ed6c6c28cd 100644 > > --- a/src/esx/esx_vi.c > > +++ b/src/esx/esx_vi.c > > @@ -433,14 +429,13 @@ int > > esxVI_CURL_Upload(esxVI_CURL *curl, const char *url, const char *content) > > { > > int responseCode = 0; > > +g_autoptr(GMutexLocker) locker = g_mutex_locker_new(&curl->lock); > > > > if (!content) { > > virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid > > argument")); > > return -1; > > } > > > > -virMutexLock(&curl->lock); > > - > > Careful #2 here about locking earlier: while usually this is not an > issue, it could be in case the code that was executed without the lock > held can be called by other code branches with the lock held. > > Again, this must be thoroughly checked in the whole patch series. I'll recheck but I tried to do it in places where it was obvious the lock was not held because of an early return case. Att. -- Rafael Fonseca
FYI: Report about libvirt failing to compile on MSYS2 with an XDR error
.. is here: https://github.com/msys2/MINGW-packages/issues/6384 Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
Re: [libvirt PATCH 0/2] Improve entry point for new contributors
On Mon, 2020-04-06 at 12:07 +0200, Andrea Bolognani wrote: > Use the path most people will look for, reduce building instructions > to the bare minimum necessary to locally bootstrap the learning > process, and ensure duplication doesn't creep in over time by > pointing to the full documentation. > > Andrea Bolognani (2): > CONTRIBUTING: Add entry point for new contributors > README-hacking: Drop from the git repository > > CONTRIBUTING.md | 18 > README-hacking | 56 - > 2 files changed, 18 insertions(+), 56 deletions(-) > create mode 100644 CONTRIBUTING.md > delete mode 100644 README-hacking ping -- Andrea Bolognani / Red Hat / Virtualization