Re: [libvirt] [ruby PATCH] Fix default values for node_cpu_stats() and node_memory_stats()
On 11/14/19 4:57 PM, Cole Robinson wrote: > On 10/22/19 11:47 AM, Stefano Garzarella wrote: >> ruby_libvirt_value_to_int() returns 0 if the optional value is >> not defined, but in node_cpu_stats() and node_memory_stats() >> the default value of cpuNum and cellNum is -1. >> > > I know nothing about ruby or the libvirt bindings, but what you describe > makes sense and matches my reading of the code. -1 is > VIR_NODE_CPU_STATS_ALL_CPUS and VIR_NODE_MEMORY_STATS_ALL_CELLS so it > makes sense that would be the default. > > Reviewed-by: Cole Robinson > > CCing clalance who is the primary ruby-libvirt maintainer, but if he > doesn't get to it by next week then I'll figure out how to build test > it, and push > I've pushed this now - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [edk2-discuss] [OVMF] resource assignment fails for passthrough PCI GPU
(+Jiri, +libvir-list) On Fri, Nov 22, 2019 at 04:58:25PM +, Dr. David Alan Gilbert wrote: > * Laszlo Ersek (ler...@redhat.com) wrote: > > (+Dave, +Eduardo) > > > > On 11/22/19 00:00, dann frazier wrote: > > > On Tue, Nov 19, 2019 at 06:06:15AM +0100, Laszlo Ersek wrote: > > >> On 11/19/19 01:54, dann frazier wrote: > > >>> On Fri, Nov 15, 2019 at 11:51:18PM +0100, Laszlo Ersek wrote: > > On 11/15/19 19:56, dann frazier wrote: > > > Hi, > > > I'm trying to passthrough an Nvidia GPU to a q35 KVM guest, but UEFI > > > is failing to allocate resources for it. I have no issues if I boot w/ > > > a legacy BIOS, and it works fine if I tell the linux guest to do the > > > allocation itself - but I'm looking for a way to make this work w/ > > > OVMF by default. > > > > > > I posted a debug log here: > > > https://bugs.launchpad.net/ubuntu/+source/edk2/+bug/1849563/+attachment/5305740/+files/q35-uefidbg.log > > > > > > Linux guest lspci output is also available for both seabios/OVMF > > > boots here: > > > https://bugs.launchpad.net/ubuntu/+source/edk2/+bug/1849563 > > > > By default, OVMF exposes such a 64-bit MMIO aperture for PCI MMIO BAR > > allocation that is 32GB in size. The generic PciBusDxe driver collects, > > orders, and assigns / allocates the MMIO BARs, but it can work only out > > of the aperture that platform code advertizes. > > > > Your GPU's region 1 is itself 32GB in size. Given that there are > > further > > PCI devices in the system with further 64-bit MMIO BARs, the default > > aperture cannot accommodate everything. In such an event, PciBusDxe > > avoids assigning the largest BARs (to my knowledge), in order to > > conserve the most aperture possible, for other devices -- hence break > > the fewest possible PCI devices. > > > > You can control the aperture size from the QEMU command line. You can > > also do it from the libvirt domain XML, technically speaking. The knob > > is experimental, so no stability or compatibility guarantees are made. > > (That's also the reason why it's a bit of a hack in the libvirt domain > > XML.) > > > > The QEMU cmdline options is described in the following edk2 commit > > message: > > > > https://github.com/tianocore/edk2/commit/7e5b1b670c38 > > >>> > > >>> Hi Laszlo, > > >>> > > >>> Thanks for taking the time to describe this in detail! The -fw_cfg > > >>> option did avoid the problem for me. > > >> > > >> Good to hear, thanks. > > >> > > >>> I also noticed that the above > > >>> commit message mentions the existence of a 24GB card as a reasoning > > >>> behind choosing the 32GB default aperture. From what you say below, I > > >>> understand that bumping this above 64GB could break hosts w/ <= 37 > > >>> physical address bits. > > >> > > >> Right. > > >> > > >>> What would be the downside of bumping the > > >>> default aperture to, say, 48GB? > > >> > > >> The placement of the aperture is not trivial (please see the code > > >> comments in the linked commit). The base address of the aperture is > > >> chosen so that the largest BAR that can fit in the aperture may be > > >> naturally aligned. (BARs are whole powers of two.) > > >> > > >> The largest BAR that can fit in a 48 GB aperture is 32 GB. Therefore > > >> such an aperture would be aligned at 32 GB -- the lowest base address > > >> (dependent on guest RAM size) would be 32 GB. Meaning that the aperture > > >> would end at 32 + 48 = 80 GB. That still breaches the 36-bit phys > > >> address width. > > >> > > >> 32 GB is the largest aperture size that can work with 36-bit phys > > >> address width; that's the aperture that ends at 64 GB exactly. > > > > > > Thanks, yeah - now that I read the code comments that is clear (as > > > clear as it can be w/ my low level of base knowledge). In the commit you > > > mention Gerd (CC'd) had suggested a heuristic-based approach for > > > sizing the aperture. When you say "PCPU address width" - is that a > > > function of the available physical bits? > > > > "PCPU address width" is not a "function" of the available physical bits > > -- it *is* the available physical bits. "PCPU" simply stands for > > "physical CPU". > > > > > IOW, would that approach > > > allow OVMF to automatically grow the aperture to the max ^2 supported > > > by the host CPU? > > > > Maybe. > > > > The current logic in OVMF works from the guest-physical address space > > size -- as deduced from multiple factors, such as the 64-bit MMIO > > aperture size, and others -- towards the guest-CPU (aka VCPU) address > > width. The VCPU address width is important for a bunch of other purposes > > in the firmware, so OVMF has to calculate it no matter what. > > > > Again, the current logic is to calculate the highest guest-physical > > address, and then deduce the VCPU address width from that (and then > >
Re: [libvirt] [PATCH 2/2] util: virNetDevBandwidthPlug: Drop ATTRIBUTE_UNUSED(4)
On 11/22/19 1:09 PM, Erik Skultety wrote: Since we know for sure that the @bandwidth parameter is properly handled in networkCheckBandwidth (@ifaceBand), it cannot ever be NULL, but coverity doesn't see this fact. In order to prevent coverity from reporting a false positive here, drop ATTRIBUTE_UNUSED for this specific argument - virNetDevBandwidthPlug is also called from a single place only Signed-off-by: Erik Skultety --- Reviewed-by: Daniel Henrique Barboza -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Revert "network: Check for QOS before blindly using it"
On 11/22/19 1:08 PM, Erik Skultety wrote: This reverts commit f4db846c32c0a1e99a0f62b340273e48f8a98ed3. This patch results in the following error when trying to start essentially any VM with default network: unsupported configuration: QOS must be defined for network 'default' Coverity didn't see that the bandwidth == NULL it complained about in virNetDevBandwidthPlug was already checked properly in networkCheckBandwidth, thus causing networkPlugBandwidth to return 0 and finish before a call to virNetDevBandwidthPlug would have been even made. Signed-off-by: Erik Skultety --- Reviewed-by: Daniel Henrique Barboza -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 8/9] wip: start virtiofsd
On Mon, Nov 04, 2019 at 10:06:40AM +0100, Stefan Hajnoczi wrote:
> On Fri, Nov 1, 2019 at 1:18 PM Ján Tomko wrote:
> > +if (qemuSecuritySetDaemonSocketLabel(driver->securityManager, vm->def)
> > < 0)
> > +goto cleanup;
> > +fd = qemuOpenChrChardevUNIXSocket(chrdev);
> > +if (fd < 0)
> > +goto cleanup;
> > +if (qemuSecurityClearSocketLabel(driver->securityManager, vm->def) < 0)
> > +goto cleanup;
>
> qemuSecurityClearSocketLabel() is not called in the
> qemuOpenChrChardevUNIXSocket() error code path. Is this correct?
>
> > +static void
> > +qemuExtVirtioFSdStop(virQEMUDriverPtr driver,
> > + virDomainObjPtr vm,
> > + virDomainFSDefPtr fs)
> > +{
>
> The daemon stops automatically when the vhost-user socket is closed by
> QEMU. Is it necessary to implement an explicit stop function?
That's good, but we've generally wanted to be explicit about cleaning
things up to cope with unexpected circumstances. In particular QEMU
can get itself stuck as a zombie if there's a dead disk, so it is
worth tearing down virtiofsd explicitly.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 8/9] wip: start virtiofsd
On Fri, Nov 01, 2019 at 01:16:06PM +0100, Ján Tomko wrote:
> Start virtiofsd for each device using it.
>
> Pre-create the socket for communication with QEMU and pass it
> to virtiofsd.
>
> Note that virtiofsd needs to run as root.
> ---
> src/conf/domain_conf.h| 1 +
> src/qemu/qemu_extdevice.c | 191 ++
> tests/qemuxml2argvtest.c | 6 ++
> 3 files changed, 198 insertions(+)
>
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 54a7e7c52f..78f88a0c2f 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -817,6 +817,7 @@ struct _virDomainFSDef {
> unsigned long long space_soft_limit; /* in bytes */
> bool symlinksResolved;
> virDomainVirtioOptionsPtr virtio;
> +char *vhost_user_fs_path; /* TODO put this in private data */
> };
IIUC, this is a socket rather than a file, so I'd call it
"vhostuser_fs_sock" personally.
> diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
> index 463f76c21a..cb44ca325f 100644
> --- a/src/qemu/qemu_extdevice.c
> +++ b/src/qemu/qemu_extdevice.c
> @@ -20,6 +20,7 @@
>
> #include
>
> +#include "qemu_command.h"
> #include "qemu_extdevice.h"
> #include "qemu_vhost_user_gpu.h"
> #include "qemu_domain.h"
> @@ -149,6 +150,178 @@ qemuExtDevicesCleanupHost(virQEMUDriverPtr driver,
> qemuExtTPMCleanupHost(def);
> }
>
> +static char *
> +qemuExtVirtioFSCreatePidFilename(virQEMUDriverConfigPtr cfg,
> + const virDomainDef *def,
> + const char *alias)
> +{
> +g_autofree char *shortName = NULL;
> +g_autofree char *name = NULL;
> +
> +if (!(shortName = virDomainDefGetShortName(def)) ||
> +virAsprintf(, "%s-%s-virtiofsd", shortName, alias) < 0)
> +return NULL;
g_strdup_printf so we can assume this always succeeds
> +
> +return virPidFileBuildPath(cfg->stateDir, name);
> +}
likewise we can assume this aborts on oom
> +
> +
> +static char *
> +qemuExtVirtioFSCreateSocketFilename(virQEMUDriverConfigPtr cfg,
> +const virDomainDef *def,
> +const char *alias)
> +{
> +g_autofree char *shortName = NULL;
> +g_autofree char *name = NULL;
> +
> +if (!(shortName = virDomainDefGetShortName(def)) ||
> +virAsprintf(, "%s-%s-virtiofsd", shortName, alias) < 0)
> +return NULL;
> +
> +return virFileBuildPath(cfg->stateDir, name, ".sock");
> +}
Same comments
> +static int
> +qemuExtVirtioFSdStart(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> + virDomainFSDefPtr fs)
> +{
> +g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
> +g_autoptr(virCommand) cmd = NULL;
> +g_autofree char *pidfile = NULL;
> +char errbuf[1024] = { 0 };
> +virDomainChrSourceDefPtr chrdev = virDomainChrSourceDefNew(NULL);
> +pid_t pid = (pid_t) -1;
> +VIR_AUTOCLOSE errfd = -1;
> +VIR_AUTOCLOSE fd = -1;
> +int exitstatus = 0;
> +int cmdret = 0;
> +int ret = -1;
> +int rc;
> +
> +chrdev->type = VIR_DOMAIN_CHR_TYPE_UNIX;
> +chrdev->data.nix.listen = true;
> +
> +if (!(pidfile = qemuExtVirtioFSCreatePidFilename(cfg, vm->def,
> fs->info.alias)))
> +goto cleanup;
> +
> +if (!(chrdev->data.nix.path = qemuExtVirtioFSCreateSocketFilename(cfg,
> vm->def, fs->info.alias)))
> +goto cleanup;
No ned to check for failure of these two.
> +
> +if (qemuSecuritySetDaemonSocketLabel(driver->securityManager, vm->def) <
> 0)
> +goto cleanup;
> +fd = qemuOpenChrChardevUNIXSocket(chrdev);
> +if (fd < 0)
> +goto cleanup;
> +if (qemuSecurityClearSocketLabel(driver->securityManager, vm->def) < 0)
> +goto cleanup;
> +
> +/* TODO: do not call this here */
> +virDomainChrDef chr = { .source = chrdev };
> +if (qemuSecuritySetChardevLabel(driver, vm, ) < 0)
> +goto cleanup;
Indeed, needs to be in the security manager code.
> +
> +/* TODO: configure path */
> +if (!(cmd = virCommandNew("/usr/libexec/virtiofsd")))
> +goto cleanup;
In $QEMU/docs/interop/vhost-user.json there's a specificiation for
how we're expected to locate the virtiofsd binary based on metadata
files. Same approach we're using for locating firmware files
basically.
> +
> +virCommandSetPidFile(cmd, pidfile);
> +virCommandSetErrorFD(cmd, );
> +virCommandDaemonize(cmd);
> +
> +virCommandAddArg(cmd, "--syslog");
I feel like maybe we should be using a logfile as we do for
QEMU, via virtlogd ?
> +virCommandAddArgFormat(cmd, "--fd=%d", fd);
> +virCommandPassFD(cmd, fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
> +fd = -1;
> +
> +virCommandAddArg(cmd, "-o");
> +/* TODO: validate path? */
> +virCommandAddArgFormat(cmd, "source=%s", fs->src->path);
> +virCommandAddArg(cmd, "-d");
IIRC, it was decided intended
Re: [libvirt] [PATCH 6/9] conf: qemu: add virtio-fs fsdriver type
On Wed, Nov 20, 2019 at 09:38:15AM +0100, Ján Tomko wrote: > On Mon, Nov 04, 2019 at 09:56:28AM +0100, Stefan Hajnoczi wrote: > > On Fri, Nov 1, 2019 at 1:15 PM Ján Tomko wrote: > > > > > > Introduce a new 'virtio-fs' driver type for filesystem. > > > > > > > > > > > > > > > > > > > What happens with the target dir? > > > > For virtio-fs, it is used the same way as with 9pfs - it is passed as > the tag and meant as a suggestion for the guest for where to mount the > filesystem. > > For LXC, libvirt actually does the mounting so the target dir path is > honored. > > I could use some other example in the documentation that does not look > as a path if that's too confusing. I'm not sure whether deviating > from the existing pattern and using something like: > > is worth it. Yeah, we shouldn't have called the attribute "dir", but since we have that naming for ages now, we shouldn't change it IMHO. Just document that its a stupid historical name :-) Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/4] domain_conf: allow address type='none' to unassign PCI hostdevs
On 11/22/19 12:13 PM, Daniel P. Berrangé wrote: On Thu, Nov 21, 2019 at 07:19:14PM -0300, Daniel Henrique Barboza wrote: [...] The use of a new 'unassigned' flag in virDomainHostdevDef makes it easier to distinguish between the case we're handling here versus the case in which a PCI hostdev has no element. Both are interpreted as VIR_DOMAIN_DEF_FORMAT_ADDRESS_NONE in the existing code, thus reducing the logic to a flag makes it easier to handle this new use case. In the next patch we'll use the 'unassigned' flag to filter the hostdevs being initialized in the QEMU command line. This paragraph illustrates why I think this approach is a bad idea. It is overloading ADDRESS_NONE to have 2 separate meanings. When we've done such things in the past we've usually ended up regretting it. We're using ADDRESS_NONE as our signal that the user does not care about the address and thus libvirt should automatically assign one. That's consistent across all the different types of devices that we have. I don't want to see us add special semantics for this just for host devices. The fact that you stil lneeded a "bool unassigned" flag to be stored in the virDomainHostdevDef is a clear sign that this must be exposed as a distinct XML attribute. I see you in fact did exactly this in the previous version of this patch series, so I'd like to go back to that version. Do you suggest to keep exactly like it was in v1, an extra parameter in the subsys element? We can for example create a new address type, "unassigned", to avoid overloading ADDRESS_NONE like I'm doing here. I'm suggesting this because, back in v1, I had to take a peak in the element to avoid the case where the user would assign an address to an unassigned device. Since I'll need to took in the element anyway, a new "unassigned" address type would make it easier to cover this cases. It's also a bit less verbose than adding "assigned='yes|no'" in the end of the element. I could also say that this would also be easier for other device types to use, but I can't come up with a valid use case for anything other than a PCI hostdev to be unassignable. Thanks, DHB Regards, Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] Revert "network: Check for QOS before blindly using it"
This reverts commit f4db846c32c0a1e99a0f62b340273e48f8a98ed3.
This patch results in the following error when trying to start
essentially any VM with default network:
unsupported configuration: QOS must be defined for network 'default'
Coverity didn't see that the bandwidth == NULL it complained about in
virNetDevBandwidthPlug was already checked properly in
networkCheckBandwidth, thus causing networkPlugBandwidth to return 0
and finish before a call to virNetDevBandwidthPlug would have been even
made.
Signed-off-by: Erik Skultety
---
src/network/bridge_driver.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 9c49c70564..68bb916501 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -4567,13 +4567,6 @@ networkAllocatePort(virNetworkObjPtr obj,
return -1;
}
-if (!port->bandwidth) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("QOS must be defined for network '%s'"),
- netdef->name);
-return -1;
-}
-
if (networkPlugBandwidth(obj, >mac, port->bandwidth,
>class_id) < 0)
return -1;
break;
@@ -4640,13 +4633,6 @@ networkAllocatePort(virNetworkObjPtr obj,
}
}
-if (!port->bandwidth) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("QOS must be defined for network '%s'"),
- netdef->name);
-return -1;
-}
-
if (networkPlugBandwidth(obj, >mac, port->bandwidth,
>class_id) < 0)
return -1;
break;
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/2] Fix a VM startup failure: QOS must be defined for network 'default'
Caused by patching a coverity false positive in commit f4db846c32c0a1e99a0f62b340273e48f8a98ed3. Erik Skultety (2): Revert "network: Check for QOS before blindly using it" util: virNetDevBandwidthPlug: Drop ATTRIBUTE_UNUSED(4) src/network/bridge_driver.c | 14 -- src/util/virnetdevbandwidth.h | 3 +-- 2 files changed, 1 insertion(+), 16 deletions(-) -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] util: virNetDevBandwidthPlug: Drop ATTRIBUTE_UNUSED(4)
Since we know for sure that the @bandwidth parameter is properly handled in networkCheckBandwidth (@ifaceBand), it cannot ever be NULL, but coverity doesn't see this fact. In order to prevent coverity from reporting a false positive here, drop ATTRIBUTE_UNUSED for this specific argument - virNetDevBandwidthPlug is also called from a single place only Signed-off-by: Erik Skultety --- src/util/virnetdevbandwidth.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util/virnetdevbandwidth.h b/src/util/virnetdevbandwidth.h index 19323c5ed2..59d7513286 100644 --- a/src/util/virnetdevbandwidth.h +++ b/src/util/virnetdevbandwidth.h @@ -55,8 +55,7 @@ int virNetDevBandwidthPlug(const char *brname, const virMacAddr *ifmac_ptr, virNetDevBandwidthPtr bandwidth, unsigned int id) -ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4) -G_GNUC_WARN_UNUSED_RESULT; +ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) G_GNUC_WARN_UNUSED_RESULT; int virNetDevBandwidthUnplug(const char *brname, unsigned int id) -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/1] Improve travis CI coverage
On Fri, Nov 22, 2019 at 3:58 PM Daniel P. Berrangé wrote: > > There is currently a serious hardware outage with the host running > our Jenkins CI slaves and we're not sure when we'll get it back up > and running. Ths leaves us with Travis CI in the meanwhile. > > This expands the Travis CI coverage to add latest Fedora stable > and rawhide, so that it is covering a representative set of > distros overall. > > Daniel P. Berrangé (1): > travis: add fedora-31 & fedora-rawhide to the build images > > .travis.yml | 14 ++ > 1 file changed, 14 insertions(+) Reviewed-by: Fabiano Fidêncio -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/4] domain_conf: allow address type='none' to unassign PCI hostdevs
On Thu, Nov 21, 2019 at 07:19:14PM -0300, Daniel Henrique Barboza wrote: > Today, to use a PCI hostdev "A" in a domain, all PCI devices > that belongs to the same IOMMU group must also be declared in > the domain XML, meaning that all IOMMU devices are detached > from the host and all of them are visible to the guest. > > The result is that the guest will have access to all devices, > but this is not necessarily what the administrator wanted. If only > the hostdev "A" was intended for guest usage, but hostdevs "B" and > "C" happened to be in the same IOMMU group of "A", the guest will > gain access to all 3 devices. This makes the administrator rely > on alternative solutions, such as use all hostdevs with un-managed > mode and detached all the IOMMU before the guest starts. If > use un-managed mode is not an option, the only alternative left is > an ACS patch to deny guest access to "B" and "C". > > Discussions made in [1] and [2] led to the approach implemented > here. This patch builds upon the already existing > VIR_DOMAIN_DEF_FORMAT_ADDRESS_NONE format to use it as a indication > of whether a hostdev will be owned by a domain, but not visible > to the guest OS. This allows the administrator to declare all the > IOMMU while also choosing which hostdevs will be usable by the > guest. This new mechanic applies to all PCI hostdevs, regardless > of whether they are a PCI multifunction hostdev or not. Using > in any case other than a PCI hostdev > will result in error. > > The use of a new 'unassigned' flag in virDomainHostdevDef makes > it easier to distinguish between the case we're handling here > versus the case in which a PCI hostdev has no element. > Both are interpreted as VIR_DOMAIN_DEF_FORMAT_ADDRESS_NONE in > the existing code, thus reducing the logic to a flag makes > it easier to handle this new use case. In the next patch we'll > use the 'unassigned' flag to filter the hostdevs being > initialized in the QEMU command line. This paragraph illustrates why I think this approach is a bad idea. It is overloading ADDRESS_NONE to have 2 separate meanings. When we've done such things in the past we've usually ended up regretting it. We're using ADDRESS_NONE as our signal that the user does not care about the address and thus libvirt should automatically assign one. That's consistent across all the different types of devices that we have. I don't want to see us add special semantics for this just for host devices. The fact that you stil lneeded a "bool unassigned" flag to be stored in the virDomainHostdevDef is a clear sign that this must be exposed as a distinct XML attribute. I see you in fact did exactly this in the previous version of this patch series, so I'd like to go back to that version. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/1] Improve travis CI coverage
There is currently a serious hardware outage with the host running our Jenkins CI slaves and we're not sure when we'll get it back up and running. Ths leaves us with Travis CI in the meanwhile. This expands the Travis CI coverage to add latest Fedora stable and rawhide, so that it is covering a representative set of distros overall. Daniel P. Berrangé (1): travis: add fedora-31 & fedora-rawhide to the build images .travis.yml | 14 ++ 1 file changed, 14 insertions(+) -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/1] travis: add fedora-31 & fedora-rawhide to the build images
The CentOS7 distro is quite old and the Ubuntu 18.04 distro is already a year & half old. Adding a Fedora 31 image gives us coverage of the newest stable distro release, and fedora-rawhide gives us the cutting edge. Signed-off-by: Daniel P. Berrangé --- .travis.yml | 14 ++ 1 file changed, 14 insertions(+) diff --git a/.travis.yml b/.travis.yml index 8b70c1c937..75f32e53b9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -31,6 +31,20 @@ matrix: - MAKE_ARGS="syntax-check distcheck" script: - make -C ci/ ci-build@$IMAGE CI_MAKE_ARGS="$MAKE_ARGS" +- services: +- docker + env: +- IMAGE="fedora-31" +- MAKE_ARGS="syntax-check distcheck" + script: +- make -C ci/ ci-build@$IMAGE CI_MAKE_ARGS="$MAKE_ARGS" +- services: +- docker + env: +- IMAGE="fedora-rawhide" +- MAKE_ARGS="syntax-check distcheck" + script: +- make -C ci/ ci-build@$IMAGE CI_MAKE_ARGS="$MAKE_ARGS" - services: - docker env: -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 14/15] docs: convert kbase/locking-sanlock.html.in to RST
This is a semi-automated conversion. The first conversion is done using "pandoc -f html -t rst". The result is then editted manually to apply the desired heading markup, and fix a few things that pandoc gets wrong. Signed-off-by: Daniel P. Berrangé --- docs/kbase/locking-sanlock.html.in | 247 - docs/kbase/locking-sanlock.rst | 193 ++ 2 files changed, 193 insertions(+), 247 deletions(-) delete mode 100644 docs/kbase/locking-sanlock.html.in create mode 100644 docs/kbase/locking-sanlock.rst diff --git a/docs/kbase/locking-sanlock.html.in b/docs/kbase/locking-sanlock.html.in deleted file mode 100644 index 3ee9849e87..00 --- a/docs/kbase/locking-sanlock.html.in +++ /dev/null @@ -1,247 +0,0 @@ - - -http://www.w3.org/1999/xhtml;> - -Virtual machine lock manager, sanlock plugin - - - - - This page describes use of the - https://fedorahosted.org/sanlock/;>sanlock - service as a lock driver - plugin for virtual machine disk mutual exclusion. - - -Sanlock daemon setup - - - On many operating systems, the sanlock plugin - is distributed in a sub-package which needs to be installed - separately from the main libvirt RPM. On a Fedora/RHEL host - this can be done with the yum command - - - -$ su - root -# yum install libvirt-lock-sanlock - - - - The next step is to start the sanlock daemon. For maximum - safety sanlock prefers to have a connection to a watchdog - daemon. This will cause the entire host to be rebooted in - the event that sanlock crashes / terminates abnormally. - To start the watchdog daemon on a Fedora/RHEL host - the following commands can be run: - - - -$ su - root -# chkconfig wdmd on -# service wdmd start - - - - Once the watchdog is running, sanlock can be started - as follows - - - -# chkconfig sanlock on -# service sanlock start - - - - Note: if you wish to avoid the use of the - watchdog, add the following line to /etc/sysconfig/sanlock - before starting it - - - -SANLOCKOPTS="-w 0" - - - - The sanlock daemon must be started on every single host - that will be running virtual machines. So repeat these - steps as necessary. - - -libvirt sanlock plugin configuration - - - Once the sanlock daemon is running, the next step is to - configure the libvirt sanlock plugin. There is a separate - configuration file for each libvirt driver that is using - sanlock. For QEMU, we will edit /etc/libvirt/qemu-sanlock.conf - There is one mandatory parameter that needs to be set, - the host_id. This is an integer between - 1 and 2000, which must be set to a unique - value on each host running virtual machines. - - - -$ su - root -# augtool -s set /files/etc/libvirt/qemu-sanlock.conf/host_id 1 - - - - Repeat this on every host, changing 1 to a - unique value for the host. - - -libvirt sanlock storage configuration - - - The sanlock plugin needs to create leases in a directory - that is on a filesystem shared between all hosts running - virtual machines. Obvious choices for this include NFS - or GFS2. The libvirt sanlock plugin expects its lease - directory be at /var/lib/libvirt/sanlock - so update the host's /etc/fstab to mount - a suitable shared/cluster filesystem at that location - - - -$ su - root -# echo "some.nfs.server:/export/sanlock /var/lib/libvirt/sanlock nfs hard,nointr 0 0" >> /etc/fstab -# mount /var/lib/libvirt/sanlock - - - - If your sanlock daemon happen to run under non-root - privileges, you need to tell this to libvirt so it - chowns created files correctly. This can be done by - setting user and/or group - variables in the configuration file. Accepted values - range is specified in description to the same - variables in /etc/libvirt/qemu.conf. For - example: - - - -augtool -s set /files/etc/libvirt/qemu-sanlock.conf/user sanlock -augtool -s set /files/etc/libvirt/qemu-sanlock.conf/group sanlock - - - - But remember, that if this is NFS share, you need a - no_root_squash-ed one for chown (and chmod possibly) - to succeed. - - - - In terms of storage requirements, if the filesystem - uses 512 byte sectors, you need to allow for 1MB - of storage for each guest disk. So if you have a network - with 20 virtualization hosts, each running 50 virtual - machines and an average of 2 disks per guest, you will - need 20*50*2 == 2000 MB of storage for - sanlock. - - - - - On one of the hosts on the network is it wise to setup - a cron job which runs the virt-sanlock-cleanup - script periodically. This scripts deletes any lease - files which are not currently in
[libvirt] [PATCH v2 13/15] docs: convert kbase/locking-lockd.html.in to RST
This is a semi-automated conversion. The first conversion is done using "pandoc -f html -t rst". The result is then editted manually to apply the desired heading markup, and fix a few things that pandoc gets wrong. Signed-off-by: Daniel P. Berrangé --- docs/kbase/locking-lockd.html.in | 160 --- docs/kbase/locking-lockd.rst | 121 +++ 2 files changed, 121 insertions(+), 160 deletions(-) delete mode 100644 docs/kbase/locking-lockd.html.in create mode 100644 docs/kbase/locking-lockd.rst diff --git a/docs/kbase/locking-lockd.html.in b/docs/kbase/locking-lockd.html.in deleted file mode 100644 index 855404ac97..00 --- a/docs/kbase/locking-lockd.html.in +++ /dev/null @@ -1,160 +0,0 @@ - - -http://www.w3.org/1999/xhtml;> - -Virtual machine lock manager, virtlockd plugin - - - - - This page describes use of the virtlockd - service as a lock driver - plugin for virtual machine disk mutual exclusion. - - -virtlockd background - - - The virtlockd daemon is a single purpose binary which - focuses exclusively on the task of acquiring and holding - locks on behalf of running virtual machines. It is - designed to offer a low overhead, portable locking - scheme can be used out of the box on virtualization - hosts with minimal configuration overheads. It makes - use of the POSIX fcntl advisory locking capability - to hold locks, which is supported by the majority of - commonly used filesystems. - - -virtlockd daemon setup - - - In most OS, the virtlockd daemon itself will not require - any upfront configuration work. It is installed by default - when libvirtd is present, and a systemd socket unit is - registered such that the daemon will be automatically - started when first required. With OS that predate systemd - though, it will be necessary to start it at boot time, - prior to libvirtd being started. On RHEL/Fedora distros, - this can be achieved as follows - - - -# chkconfig virtlockd on -# service virtlockd start - - - - The above instructions apply to the instance of virtlockd - that runs privileged, and is used by the libvirtd daemon - that runs privileged. If running libvirtd as an unprivileged - user, it will always automatically spawn an instance of - the virtlockd daemon unprivileged too. This requires no - setup at all. - - -libvirt lockd plugin configuration - - - Once the virtlockd daemon is running, or setup to autostart, - the next step is to configure the libvirt lockd plugin. - There is a separate configuration file for each libvirt - driver that is using virtlockd. For QEMU, we will edit - /etc/libvirt/qemu-lockd.conf - - - - The default behaviour of the lockd plugin is to acquire locks - directly on the virtual disk images associated with the guest - disk elements. This ensures it can run out of the box - with no configuration, providing locking for disk images on - shared filesystems such as NFS. It does not provide any cross - host protection for storage that is backed by block devices, - since locks acquired on device nodes in /dev only apply within - the host. It may also be the case that the filesystem holding - the disk images is not capable of supporting fcntl locks. - - - - To address these problems it is possible to tell lockd to - acquire locks on an indirect file. Essentially lockd will - calculate the SHA256 checksum of the fully qualified path, - and create a zero length file in a given directory whose - filename is the checksum. It will then acquire a lock on - that file. Assuming the block devices assigned to the guest - are using stable paths (eg /dev/disk/by-path/XXX) then - this will allow for locks to apply across hosts. This - feature can be enabled by setting a configuration setting - that specifies the directory in which to create the lock - files. The directory referred to should of course be - placed on a shared filesystem (eg NFS) that is accessible - to all hosts which can see the shared block devices. - - - -$ su - root -# augtool -s set \ - /files/etc/libvirt/qemu-lockd.conf/file_lockspace_dir \ - "/var/lib/libvirt/lockd/files" - - - - If the guests are using either LVM and SCSI block devices - for their virtual disks, there is a unique identifier - associated with each device. It is possible to tell lockd - to use this UUID as the basis for acquiring locks, rather - than the SHA256 sum of the filename. The benefit of this - is that the locking protection will work even if the file - paths to the given block device are different on each - host. - - - -$ su - root -# augtool -s set \ -
[libvirt] [PATCH v2 05/15] docs: generate permalinks correctly for rst2html output
The rst2html output generates the links for headings in a slightly
different way than we do for docs written in HTML, so we must match
another scenario when generating back links.
rst2html will also use tags for both the document title and
the first level of section titles, so we must expand the matching
to allow for this too.
Signed-off-by: Daniel P. Berrangé
---
docs/libvirt.css | 1 +
docs/page.xsl| 5 -
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/docs/libvirt.css b/docs/libvirt.css
index e927a084a7..399404ca54 100644
--- a/docs/libvirt.css
+++ b/docs/libvirt.css
@@ -419,6 +419,7 @@ a.headerlink {
visibility: hidden;
}
+h1:hover > a.headerlink,
h2:hover > a.headerlink,
h3:hover > a.headerlink,
h4:hover > a.headerlink,
diff --git a/docs/page.xsl b/docs/page.xsl
index 70dfec6df6..f8f7ff8cf9 100644
--- a/docs/page.xsl
+++ b/docs/page.xsl
@@ -176,12 +176,15 @@
-
+
+
+
+
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 15/15] docs: add a kbase page about RPM packaging options
The libvirt RPM packaging is quite fine grained but it is not obvious to users which package is best to install. Add a kbase doc that describes the different RPMs, and illustrates some example deployment use cases. Signed-off-by: Daniel P. Berrangé --- docs/kbase.html.in| 4 + docs/kbase/rpm-deployment.rst | 410 ++ 2 files changed, 414 insertions(+) create mode 100644 docs/kbase/rpm-deployment.rst diff --git a/docs/kbase.html.in b/docs/kbase.html.in index 97d3f4c384..a5504a540f 100644 --- a/docs/kbase.html.in +++ b/docs/kbase.html.in @@ -21,6 +21,10 @@ capture Comparison between different methods of capturing domain state + +RPM deployment +Explanation of the different RPM packages and illustration of + which to pick for installation diff --git a/docs/kbase/rpm-deployment.rst b/docs/kbase/rpm-deployment.rst new file mode 100644 index 00..8f1584d7ea --- /dev/null +++ b/docs/kbase/rpm-deployment.rst @@ -0,0 +1,410 @@ +=== +RPM Deployment Guidance +=== + +.. contents:: + +A complete libvirt build includes a wide range of features, many of which are +dynamically loadable at runtime. Applications using libvirt typically only +need to use a subset of these features, and so do not require a full install +of all libvirt RPM packages. + +This document provides some guidance on the RPM packages available with libvirt +on Fedora and related distributions, to enable applications and administrators +to pick the optimal set for their needs. + +The RHEL and CentOS distributions use the same RPM packaging split, but many +of the drivers will be disabled at build time, so not all of the packages +listed on this page will exist. + + +RPM packages + + +* libvirt + + This is an empty package that exists solely as a convenient way to install + every other libvirt RPM package. Almost every deployment scenario would be + better served by picking one of the other RPMs listed below. + +* libvirt-admin + + The virt-admin tool, used for administrative operations on any libvirt + daemons. Most usefully it allows for logging filters and outputs to be + reconfigured on a running daemon without a restart. This is recommended + to be installed on any host running a libvirt daemon. + + +* libvirt-bash-completion + + Argument auto-completion support for the Bash shell. This is shared code that + is pulled in by either the libvirt-admin or libvirt-clients RPMs, so there is + no need to explicitly ask for this package to be installed. + + +* libvirt-client + + The virsh tool, used for interacting with any libvirt driver, both primary + virt drivers and secondary drivers for storage, networking, etc. All libvirt + installs should have this installed as it provides a useful way to view and + debug what is being done by other applications using libvirt. + + +* libvirt-daemon + + The monolithic libvirtd daemon, traditionally used for running all the + stateful drivers. This package does not contain any drivers, so further + packages need to be installed to provide the desired drivers. + + +* libvirt-daemon-config-network + + The sample configuration file providing the 'default' virtual network that + enables outbound NAT based connectivity for virtual machines. This is useful + on desktop installations, but is not typically desired on server + installations where VMs will use full bridged connectivity. + + +* libvirt-daemon-config-nwfilter + + The sample configuration files providing the network filters for protecting + against common malicious guest traffic. This includes protection against ARP, + MAC and IP spoofing. This is typically desired on server installations, if + the mgmt app is using libvirt's network filtering features. + + +* libvirt-daemon-driver-interface + + The dynamically loadable driver providing an implementation of the host + network interface management APIs, as well as the virtinterfaced daemon + binary. + + +* libvirt-daemon-driver-libxl + + The dynamically loadable driver providing an implementation of the hypervisor + APIs for Xen using the libxl library, as well as the virtxend daemon + binary. + + Note that this is a minimal package so does not actually pull in the full + Xen hypervisor package set. This be must requested separately. + + +* libvirt-daemon-driver-lxc + + The dynamically loadable driver providing an implementation of the hypervisor + APIs for Linux containers, as well as the virtlxcd daemon binary. + + +* libvirt-daemon-driver-network + + The dynamically loadable driver providing an implementation of the virtual + network interface management APIs, as well as the virtinterfaced daemon + binary. Typically the libvirt-daemon-config-network RPM will also be desired + when this is installed. + + +* libvirt-daemon-driver-nodedev + + The dynamically loadable driver providing an
[libvirt] [PATCH v2 06/15] docs: relax CSS context match for pretty tables
We currently only render pretty tables if they have the "top_table"
class set. All of our tables set this, except for the ACL & migration
doc tables, which should have set it, and the API reference which does
not want it.
Simplify life by rendering all tables in a pretty style and remove the
need for the "top_table" class entirely. A small rule turns off the
pretty style for the API reference where tables are a hack used to
render enums with horizontal alignment.
Signed-off-by: Daniel P. Berrangé
---
docs/aclpolkit.html.in | 20 +--
docs/genaclperms.pl| 2 +-
docs/libvirt.css | 75 --
docs/migration.html.in | 2 +-
docs/newapi.xsl| 4 +--
5 files changed, 35 insertions(+), 68 deletions(-)
diff --git a/docs/aclpolkit.html.in b/docs/aclpolkit.html.in
index 68e6d399b2..4a8877d5e7 100644
--- a/docs/aclpolkit.html.in
+++ b/docs/aclpolkit.html.in
@@ -64,7 +64,7 @@
virConnectPtr
-
+
Attribute
@@ -80,7 +80,7 @@
virDomainPtr
-
+
Attribute
@@ -104,7 +104,7 @@
virInterfacePtr
-
+
Attribute
@@ -128,7 +128,7 @@
virNetworkPtr
-
+
Attribute
@@ -152,7 +152,7 @@
virNodeDevicePtr
-
+
Attribute
@@ -172,7 +172,7 @@
virNWFilterPtr
-
+
Attribute
@@ -196,7 +196,7 @@
virSecretPtr
-
+
Attribute
@@ -232,7 +232,7 @@
virStoragePoolPtr
-
+
Attribute
@@ -256,7 +256,7 @@
virStorageVolPtr
-
+
Attribute
@@ -317,7 +317,7 @@
Connection Driver Name
-
+
Connection Driver
diff --git a/docs/genaclperms.pl b/docs/genaclperms.pl
index e20b4c11c3..0de2cfad4d 100755
--- a/docs/genaclperms.pl
+++ b/docs/genaclperms.pl
@@ -85,7 +85,7 @@ foreach my $object (sort { $a cmp $b } keys %perms) {
my $olink = lc "object_" . $object;
print <$class
-
+
Permission
diff --git a/docs/libvirt.css b/docs/libvirt.css
index 399404ca54..d2e1842b62 100644
--- a/docs/libvirt.css
+++ b/docs/libvirt.css
@@ -161,37 +161,37 @@ p.image {
text-align: center;
}
-.top_table {
+table {
border-collapse: collapse;
min-width: 60%;
margin-left: auto;
margin-right: auto;
}
-.top_table th {
+table th {
background: rgb(0, 95, 97);
color: rgb(255, 255, 255);
padding: 0.5em;
}
-.top_table th a {
+table th a {
color: inherit;
text-decoration: inherit;
}
-.top_table td, .top_table th {
+table td, table th {
border: 1px solid rgb(60, 133, 124);
}
-.top_table td {
+table td {
padding: 4px;
}
-.top_table tr:hover td, .top_table col:hover td {
+table tr:hover td, table col:hover td {
background: #ee;
}
-.top_table tr td:hover {
+table tr td:hover {
background: #c5dbd8;
}
@@ -289,42 +289,12 @@ img.diagram {
margin-right: auto;
}
-table.data th, table.data td {
-padding: 0.3em;
-}
-
-table.data {
-border-spacing: 0px;
-}
-
-table.data thead th {
-background: rgb(178,178,178);
-text-align: center;
-}
-
-table.data {
-border: 1px solid black;
-border-collapse: collapse;
-}
-
-table.data thead tr th {
-border: 1px solid black;
-}
-
-table.data tr.head th {
-border-left: 1px solid black;
-border-right: 1px solid black;
-}
-
-table.data tbody td {
-background: rgb(240,240,240);
-}
-table.data tbody td.y {
+table tbody td.y {
background: rgb(220,255,220);
text-align: center;
}
-table.data tbody td.n {
+table tbody td.n {
background: rgb(255,220,220);
text-align: center;
}
@@ -377,6 +347,18 @@ table.data tbody td.n {
text-decoration: none;
}
+.api table td,.api table th {
+border: 0px;
+}
+
+.api table tr:hover td, .api table col:hover td {
+background: inherit;
+}
+
+.api table tr td:hover {
+background: inherit;
+}
+
dl.variablelist > dt {
display: block;
float: left;
@@ -392,21 +374,6 @@ dl.variablelist > dt:after {
content: ": ";
}
-table.acl {
-margin: 1em;
-border-spacing: 0px;
-border: 1px solid #ccc;
-}
-
-table.acl tr, table.acl td {
-padding: 0.3em;
-border: 1px solid #ccc;
-}
-
-table.acl thead {
-background: #ddd;
-}
-
div.description pre.code {
border: 1px dashed grey;
background-color: inherit;
diff --git a/docs/migration.html.in b/docs/migration.html.in
index 7c345b65b7..355f0e89af 100644
--- a/docs/migration.html.in
+++ b/docs/migration.html.in
@@ -257,7 +257,7 @@
combinations.
-
+
Before migration
diff --git a/docs/newapi.xsl b/docs/newapi.xsl
index 670879dc48..0dc4f7ae52 100644
--- a/docs/newapi.xsl
+++
[libvirt] [PATCH v2 08/15] docs: add a minimal style guide for writing RST docs
Most importantly we document the required heading markup so that we get consistency across the docs. Also mention that docs should have a table of contents if they have headings & are likely longer than one page of text. The 3-space indent rule may sound wierd, but that's what python has recommended and thus what tools like pandoc emit. Rather than try to reindent things to 4-space, just accept this RST norm. Signed-off-by: Daniel P. Berrangé --- docs/docs.html.in | 3 +++ docs/styleguide.rst | 66 + 2 files changed, 69 insertions(+) create mode 100644 docs/styleguide.rst diff --git a/docs/docs.html.in b/docs/docs.html.in index f2721964b5..f441769617 100644 --- a/docs/docs.html.in +++ b/docs/docs.html.in @@ -135,6 +135,9 @@ Contributor guidelines General hacking guidelines for contributors +Docs style guide +Style guidelines for reStructuredText docs + Project strategy Sets a vision for future direction technical choices diff --git a/docs/styleguide.rst b/docs/styleguide.rst new file mode 100644 index 00..71f29320cb --- /dev/null +++ b/docs/styleguide.rst @@ -0,0 +1,66 @@ += +Documentation style guide += + +.. contents:: + +The following documents some specific libvirt rules for writing docs in +reStructuredText + +Table of contents += + +Any document which uses headings and whose content is long enough to cause +scrolling when viewed in the browser must start with a table of contents. +This should be created using the default formatting: + +:: + + .. contents:: + + +Whitespace +== + +Blocks should be indented with 3 spaces, and no tabs + + +Headings + + +RST allows headings to be created simply by underlining with any punctuation +characters. Optionally the text may be overlined to. + +For the sake of consistency, libvirt defines the following style requirement +which allows for 6 levels of headings + +:: + + = + Heading 1 + = + + + + Heading 2 + = + + + + Heading 3 + - + + + + Heading 4 + ~ + + + + Heading 5 + . + + + + Heading 6 + ^ -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 09/15] docs: convert kbase/domainstatecapture.html.in to RST
This is a semi-automated conversion. The first conversion is done using "pandoc -f html -t rst". The result is then editted manually to apply the desired heading markup, and fix a few things that pandoc gets wrong. Signed-off-by: Daniel P. Berrangé --- docs/kbase/domainstatecapture.html.in | 303 -- docs/kbase/domainstatecapture.rst | 255 ++ 2 files changed, 255 insertions(+), 303 deletions(-) delete mode 100644 docs/kbase/domainstatecapture.html.in create mode 100644 docs/kbase/domainstatecapture.rst diff --git a/docs/kbase/domainstatecapture.html.in b/docs/kbase/domainstatecapture.html.in deleted file mode 100644 index f8c7394785..00 --- a/docs/kbase/domainstatecapture.html.in +++ /dev/null @@ -1,303 +0,0 @@ - - -http://www.w3.org/1999/xhtml;> - - -Domain state capture using Libvirt - - - - - In order to aid application developers to choose which - operations best suit their needs, this page compares the - different means for capturing state related to a domain managed - by libvirt. - - - - The information here is primarily geared towards capturing the - state of an active domain. Capturing the state of an inactive - domain essentially amounts to copying the contents of guest - disks, followed by a fresh boot of the same domain configuration - with disks restored back to that saved state. - - -State capture trade-offs - -One of the features made possible with virtual machines is live - migration -- transferring all state related to the guest from - one host to another with minimal interruption to the guest's - activity. In this case, state includes domain memory (including - register and device contents), and domain storage (whether the - guest's view of the disks are backed by local storage on the - host, or by the hypervisor accessing shared storage over a - network). A clever observer will then note that if all state is - available for live migration, then there is nothing stopping a - user from saving some or all of that state at a given point of - time in order to be able to later rewind guest execution back to - the state it previously had. The astute reader will also realize - that state capture at any level requires that the data must be - stored and managed by some mechanism. This processing might fit - in a single file, or more likely require a chain of related - files, and may require synchronization with third-party tools - built around managing the amount of data resulting from - capturing the state of multiple guests that each use multiple - disks. - - - - There are several libvirt APIs associated with capturing the - state of a guest, which can later be used to rewind that guest - to the conditions it was in earlier. The following is a list of - trade-offs and differences between the various facets that - affect capturing domain state for active domains: - - - - Duration - Capturing state can be a lengthy process, so while the -captured state ideally represents an atomic point in time -corresponding to something the guest was actually executing, -capturing state tends to focus on minimizing guest downtime -while performing the rest of the state capture in parallel -with guest execution. Some interfaces require up-front -preparation (the state captured is not complete until the API -ends, which may be some time after the command was first -started), while other interfaces track the state when the -command was first issued, regardless of the time spent in -capturing the rest of the state. Also, time spent in state -capture may be longer than the time required for live -migration, when state must be duplicated rather than shared. - - - Amount of state - For an online guest, there is a choice between capturing the -guest's memory (all that is needed during live migration when -the storage is already shared between source and destination), -the guest's disk state (all that is needed if there are no -pending guest I/O transactions that would be lost without the -corresponding memory state), or both together. Reverting to -partial state may still be viable, but typically, booting from -captured disk state without corresponding memory is comparable -to rebooting a machine that had power cut before I/O could be -flushed. Guests may need to use proper journaling methods to -avoid problems when booting from partial state. - - - Quiescing of data - Even if a guest has no pending I/O, capturing disk state may -catch the guest at a time when the contents of the disk are -inconsistent. Cooperating
[libvirt] [PATCH v2 07/15] docs: add styling for element
Although is deprecated in HTML5, the rst2html command will still
emit it, in preference to tags, so we must style it too.
Signed-off-by: Daniel P. Berrangé
---
docs/generic.css | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/docs/generic.css b/docs/generic.css
index a6b2354df0..c4092abc2b 100644
--- a/docs/generic.css
+++ b/docs/generic.css
@@ -72,11 +72,11 @@ h6 {
font-size: 0.8em;
}
-code, pre {
+code, pre, tt {
font-family: LibvirtOverpassMono;
}
-dd code, p code {
+dd code, p code, tt {
background-color: #ee;
}
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 01/15] docs: split TLS certificate setup into its own file
The generation and deployment of x509 certificates for TLS is complex and verbose and thus deserves its own standalone page. Signed-off-by: Daniel P. Berrangé --- docs/docs.html.in | 3 + docs/remote.html.in | 408 + docs/tlscerts.html.in | 413 ++ 3 files changed, 417 insertions(+), 407 deletions(-) create mode 100644 docs/tlscerts.html.in diff --git a/docs/docs.html.in b/docs/docs.html.in index 268c16f3b3..f2721964b5 100644 --- a/docs/docs.html.in +++ b/docs/docs.html.in @@ -18,6 +18,9 @@ Remote access Enable remote access over TCP +TLS certs +Generate and deploy x509 certificates for TLS + Authentication Configure authentication for the libvirt daemon diff --git a/docs/remote.html.in b/docs/remote.html.in index 78e071a898..5a0ebe4790 100644 --- a/docs/remote.html.in +++ b/docs/remote.html.in @@ -61,7 +61,7 @@ Remote libvirt supports a range of transports: http://en.wikipedia.org/wiki/Transport_Layer_Security; title="Transport Layer Security">TLS 1.0 (SSL 3.1) authenticated and encrypted TCP/IP socket, usually listening on a public port number. To use this you will need to - generate client and + generate client and server certificates. The standard port is 16514. @@ -382,412 +382,6 @@ Note that parameter values must be Example: sshauth=privkey,agent - - Generating TLS certificates - - - Public Key Infrastructure set up - - -If you are unsure how to create TLS certificates, skip to the -next section. - - - - Location - Machine - Description - Required fields - - - - /etc/pki/CA/cacert.pem - - Installed on the client and server - CA's certificate (more info) - n/a - - - - $HOME/.pki/cacert.pem - - Installed on the client - CA's certificate (more info) - n/a - - - - /etc/pki/libvirt/private/serverkey.pem - - Installed on the server - Server's private key (more info) - n/a - - - - /etc/pki/libvirt/servercert.pem - - Installed on the server - Server's certificate signed by the CA. - (more info) - CommonName (CN) must be the hostname of the server as it - is seen by clients. All hostname and IP address variants that might - be used to reach the server should be listed in Subject Alt Name - fields. - - - - /etc/pki/libvirt/private/clientkey.pem - - Installed on the client - Client's private key. (more info) - n/a - - - - /etc/pki/libvirt/clientcert.pem - - Installed on the client - Client's certificate signed by the CA - (more info) - Distinguished Name (DN) can be checked against an access - control list (tls_allowed_dn_list). - - - - - $HOME/.pki/libvirt/clientkey.pem - - Installed on the client - Client's private key. (more info) - n/a - - - - $HOME/.pki/libvirt/clientcert.pem - - Installed on the client - Client's certificate signed by the CA - (more info) - Distinguished Name (DN) can be checked against an access - control list (tls_allowed_dn_list). - - - - - If 'pkipath' is specified in URI, then all the client - certificates must be found in the path specified, otherwise the - connection will fail with a fatal error. If 'pkipath' is not - specified: - - - For a non-root user, libvirt tries to find the certificates -in $HOME/.pki/libvirt first. If the required CA certificate cannot -be found, then the global default location -(/etc/pki/CA/cacert.pem) will be used. -Likewise, if either the client certificate -or the client key cannot be found, then the global default -locations (/etc/pki/libvirt/clientcert.pem, -/etc/pki/libvirt/private/clientkey.pem) will be used. - - For the root user, the global default locations will always be used. - - - Background to TLS certificates - - -Libvirt supports TLS certificates for verifying the identity -of the server and clients. There are two distinct checks involved: - - - The client should know that it is connecting to the right -server. Checking done by client by matching the certificate that -the server sends to the server's hostname. May be disabled by adding -?no_verify=1 to the -remote URI. - - The server should know that only permitted clients are -connecting. This can be done based on client's IP address,
[libvirt] [PATCH v2 04/15] docs: adapt filling of section for rst2html output
The HTML from rst2html doesn't have immediately under the tag, instead there is at least one in between. There are also many things added in the section that we don't want to have copied over, since our templating system already adds suitable elements. We only need to copy the
[libvirt] [PATCH v2 11/15] docs: convert kbase/secureusage.html.in to RST
This is a semi-automated conversion. The first conversion is done using "pandoc -f html -t rst". The result is then editted manually to apply the desired heading markup, and fix a few things that pandoc gets wrong. Signed-off-by: Daniel P. Berrangé --- docs/kbase/secureusage.html.in | 171 - docs/kbase/secureusage.rst | 131 + 2 files changed, 131 insertions(+), 171 deletions(-) delete mode 100644 docs/kbase/secureusage.html.in create mode 100644 docs/kbase/secureusage.rst diff --git a/docs/kbase/secureusage.html.in b/docs/kbase/secureusage.html.in deleted file mode 100644 index c60187fab3..00 --- a/docs/kbase/secureusage.html.in +++ /dev/null @@ -1,171 +0,0 @@ - - -http://www.w3.org/1999/xhtml;> - - -Secure Usage of Libvirt - - - - - This page details information that application developers and - administrators of libvirt should be aware of when working with - libvirt, that may have a bearing on security of the system. - - - -Disk image handling - -Disk image format probing - - - Historically there have been multiple flaws in QEMU and most - projects using QEMU, related to handling of disk formats. - The problems occur when a guest is given a virtual disk backed - by raw disk format on the host. If the management application - on the host tries to auto-detect / probe the disk format, it - is vulnerable to a malicious guest which can write a qcow2 - file header into its raw disk. If the management application - subsequently probes the disk, it will see it as a 'qcow2' disk - instead of a 'raw' disk. Since 'qcow2' disks can have a copy - on write backing file, such flaw can be leveraged to read - arbitrary files on the host. The same type of flaw may occur - if the management application allows users to upload pre-created - raw images. - - - - Recommendation: never attempt to automatically - detect the format of a disk image based on file contents which - are accessible to / originate from an untrusted source. - - -Disk image backing files - - - If a management application allows users to upload pre-created - disk images in non-raw formats, it can be tricked into giving - the user access to arbitrary host files via the copy-on-write - backing file feature. This is because the qcow2 disk format - header contains a filename field which can point to any location. - It can also point to network protocols such as NBD, HTTP, GlusterFS, - RBD and more. This could allow for compromise of almost arbitrary - data accessible on the LAN/WAN. - - - - Recommendation: always validate that a disk - image originating from an untrusted source has no backing - file set. If a backing file is seen, reject the image. - - -Disk image size validation - - - If an application allows users to upload pre-created disk - images in non-raw formats, it is essential to validate the - logical disk image size, rather than the physical disk - image size. Non-raw disk images have a grow-on-demand - capability, so a user can provide a qcow2 image that may - be only 1 MB in size, but is configured to grow to many - TB in size. - - - - Recommendation: if receiving a non-raw disk - image from an untrusted source, validate the logical image - size stored in the disk image metadata against some finite - limit. - - -Disk image data access - - - If an untrusted disk image is ever mounted on the host OS by - a management application or administrator, this opens an - avenue of attack with which to potentially compromise the - host kernel. Filesystem drivers in OS kernels are often very - complex code and thus may have bugs lurking in them. With - Linux, there are a large number of filesystem drivers, many - of which attract little security analysis attention. Linux - will helpfully probe filesystem formats if not told to use an - explicit format, allowing an attacker the ability to target - specific weak filesystem drivers. Even commonly used and - widely audited filesystems such as ext4 have had - https://lwn.net/Articles/538898/;>bugs lurking in them - undetected for years at a time. - - - - Recommendation: if there is a need to access - the content of a disk image, use a single-use throwaway virtual - machine to access the data. Never mount disk images on the host - OS. Ideally make use of the http://libguestfs.org;>libguestfs - tools and APIs for accessing disks - - -Guest migration network - - - Most hypervisors with support for guest migration between hosts - make use of one (or more) network connections. Typically the source - host will connect to some port on
[libvirt] [PATCH v2 12/15] docs: convert kbase/locking.html.in to RST
This is a semi-automated conversion. The first conversion is done using "pandoc -f html -t rst". The result is then editted manually to apply the desired heading markup, and fix a few things that pandoc gets wrong. Signed-off-by: Daniel P. Berrangé --- docs/kbase/locking.html.in | 48 -- docs/kbase/locking.rst | 33 ++ 2 files changed, 33 insertions(+), 48 deletions(-) delete mode 100644 docs/kbase/locking.html.in create mode 100644 docs/kbase/locking.rst diff --git a/docs/kbase/locking.html.in b/docs/kbase/locking.html.in deleted file mode 100644 index 4532dbddf9..00 --- a/docs/kbase/locking.html.in +++ /dev/null @@ -1,48 +0,0 @@ - - -http://www.w3.org/1999/xhtml;> - -Virtual machine lock manager - - - - - Libvirt includes a framework for ensuring mutual exclusion - between virtual machines using host resources. Typically - this is used to prevent two VM processes from having concurrent - write access to the same disk image, as this would result in - data corruption if the guest was not using a cluster - aware filesystem. - - -Lock manager plugins - - - The lock manager framework has a pluggable architecture, - to allow different locking technologies to be used. - - - - nop - This is a "no op" implementation which does absolutely -nothing. This can be used if mutual exclusion between -virtual machines is not required, or if it is being -solved at another level in the management stack. - lockd - This is the current preferred implementation shipped -with libvirt. It uses the virtlockd daemon -to manage locks using the POSIX fcntl() advisory locking -capability. As such it requires a shared filesystem of -some kind be accessible to all hosts which share the -same image storage. - sanlock - This is an alternative implementation preferred by -the oVirt project. It uses a disk paxos algorithm for -maintaining continuously renewed leases. In the default -setup it requires some shared filesystem, but it is -possible to use it in a manual mode where the management -application creates leases in SAN storage volumes. - - - - diff --git a/docs/kbase/locking.rst b/docs/kbase/locking.rst new file mode 100644 index 00..c0d5e39b2d --- /dev/null +++ b/docs/kbase/locking.rst @@ -0,0 +1,33 @@ + +Virtual machine lock manager + + +Libvirt includes a framework for ensuring mutual exclusion between +virtual machines using host resources. Typically this is used to prevent +two VM processes from having concurrent write access to the same disk +image, as this would result in data corruption if the guest was not +using a cluster aware filesystem. + +Lock manager plugins + + +The lock manager framework has a pluggable architecture, to allow +different locking technologies to be used. + +nop + This is a "no op" implementation which does absolutely nothing. This + can be used if mutual exclusion between virtual machines is not + required, or if it is being solved at another level in the management + stack. +`lockd `__ + This is the current preferred implementation shipped with libvirt. It + uses the ``virtlockd`` daemon to manage locks using the POSIX fcntl() + advisory locking capability. As such it requires a shared filesystem + of some kind be accessible to all hosts which share the same image + storage. +`sanlock `__ + This is an alternative implementation preferred by the oVirt project. + It uses a disk paxos algorithm for maintaining continuously renewed + leases. In the default setup it requires some shared filesystem, but + it is possible to use it in a manual mode where the management + application creates leases in SAN storage volumes. -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 10/15] docs: convert kbase/launch_security_sev.html.in to RST
This is a semi-automated conversion. The first conversion is done using "pandoc -f html -t rst". The result is then editted manually to apply the desired heading markup, and fix a few things that pandoc gets wrong. Signed-off-by: Daniel P. Berrangé --- docs/kbase/launch_security_sev.html.in | 533 - docs/kbase/launch_security_sev.rst | 529 2 files changed, 529 insertions(+), 533 deletions(-) delete mode 100644 docs/kbase/launch_security_sev.html.in create mode 100644 docs/kbase/launch_security_sev.rst diff --git a/docs/kbase/launch_security_sev.html.in b/docs/kbase/launch_security_sev.html.in deleted file mode 100644 index 985c11a47b..00 --- a/docs/kbase/launch_security_sev.html.in +++ /dev/null @@ -1,533 +0,0 @@ - - -http://www.w3.org/1999/xhtml;> - -Launch security with AMD SEV - - - - -Storage encryption in modern public cloud computing is a common practice. -However, from the point of view of a user of these cloud workloads, a -significant amount of trust needs to be put in the cloud platform security as -well as integrity (was the hypervisor tampered?). For this reason there's ever -rising demand for securing data in use, i.e. memory encryption. -One of the solutions addressing this matter is AMD SEV. - - -AMD SEV - -SEV (Secure Encrypted Virtualization) is a feature extension of AMD's SME (Secure -Memory Encryption) intended for KVM virtual machines which is supported -primarily on AMD's EPYC CPU line. In contrast to SME, SEV uses a unique memory encryption -key for each VM. The whole encryption of memory pages is completely transparent -to the hypervisor and happens inside dedicated hardware in the on-die memory controller. -Each controller includes a high-performance Advanced Encryption Standard -(AES) engine that encrypts data when it is written to DRAM and decrypts it -when read. - -For more details about the technology itself, you can visit - https://developer.amd.com/sev/;>AMD's developer portal. - - -Enabling SEV on the host - - Before VMs can make use of the SEV feature you need to make sure your - AMD CPU does support SEV. You can check whether SEV is among the CPU - flags with: - - - -$ cat /proc/cpuinfo | grep sev -... -sme ssbd sev ibpb - - - Next step is to enable SEV in the kernel, because it is disabled by default. - This is done by putting the following onto the kernel command line: - - - -mem_encrypt=on kvm_amd.sev=1 - - - - To make the changes persistent, append the above to the variable holding - parameters of the kernel command line in - /etc/default/grub to preserve SEV settings across reboots - - - -$ cat /etc/default/grub -... -GRUB_CMDLINE_LINUX="... mem_encrypt=on kvm_amd.sev=1" -$ grub2-mkconfig -o /boot/efi/EFI/distro/grub.cfg - - -mem_encrypt=on turns on the SME memory encryption feature on -the host which protects against the physical attack on the hypervisor -memory. The kvm_amd.sev parameter actually enables SEV in -the kvm module. It can be set on the command line alongside -mem_encrypt like shown above, or it can be put into a -module config under /etc/modprobe.d/ - - - -$ cat /etc/modprobe.d/sev.conf -options kvm_amd sev=1 - - - - After rebooting the host, you should see SEV being enabled in the kernel: - - - -$ cat /sys/module/kvm_amd/parameters/sev -1 - - - Checking SEV support in the virt stack - -Note: All of the commands bellow need to be run with root privileges. - - - - First make sure you have the following packages in the specified versions: - - - - -libvirt >= 4.5.0 (>5.1.0 recommended due to additional SEV bugfixes) - - -QEMU >= 2.12.0 - - - - To confirm that the virtualization stack supports SEV, run the following: - - - -# virsh domcapabilities -domainCapabilities -... - features -... -sev supported='yes' - cbitpos47/cbitpos - reducedPhysBits1/reducedPhysBits -/sev -... - /features -/domainCapabilities - - Note that if libvirt was already installed and libvirtd running before enabling SEV in the kernel followed by the host reboot you need to force libvirtd - to re-probe both the host and QEMU capabilities. First stop libvirtd: - - - -# systemctl stop libvirtd.service - - - - Now you need to clean the capabilities cache: - - - -# rm -f /var/cache/libvirt/qemu/capabilities/* - - - - If you now restart libvirtd, it will
[libvirt] [PATCH v2 02/15] docs: move docs about remote driver URIs into URI docs
The docs about remote URIs in uri.html are somewhat sparse with the full
docs being in remote.html. Move all the URI content from remote.html
into uri.html so the user only needs to look in one place for URI info.
Signed-off-by: Daniel P. Berrangé
---
docs/remote.html.in | 278 +---
docs/uri.html.in| 263 -
2 files changed, 238 insertions(+), 303 deletions(-)
diff --git a/docs/remote.html.in b/docs/remote.html.in
index 5a0ebe4790..0b0dc87f6f 100644
--- a/docs/remote.html.in
+++ b/docs/remote.html.in
@@ -34,7 +34,7 @@ the system-wide QEMU daemon on a remote machine called
qemu://compute1.libvirt.org/system.
-The section on remote URIs
+The section on remote URIs
describes in more detail these remote URIs.
@@ -109,279 +109,9 @@ even with graphical management applications. As with the
classic ssh transport
netcat is required on the remote side.
-The default transport, if no other is specified, is tls.
-
-
- Remote URIs
-
-
-See also: documentation on ordinary ("local") URIs.
-
-
-Remote URIs have the general form ("[...]" meaning an optional part):
-
-
driver[+transport]://[username@][hostname][:port]/[path][?extraparameters]
-
-
-Either the transport or the hostname must be given in order
-to distinguish this from a local URI.
-
-
-Some examples:
-
-
- xen+ssh://rjones@towada/system Connect to
a
-remote Xen hypervisor on host towada using ssh transport and ssh
-username rjones.
-
- xen://towada/system Connect to a
-remote Xen hypervisor on host towada using TLS.
-
- xen://towada/system?no_verify=1 Connect
to a
-remote Xen hypervisor on host towada using TLS. Do not verify
-the server's certificate.
-
-
qemu+unix:///system?socket=/opt/libvirt/run/libvirt/libvirt-sock
-Connect to the local qemu instances over a non-standard
-Unix socket (the full path to the Unix socket is
-supplied explicitly in this case).
-
- test+tcp://localhost:5000/default
-Connect to a libvirtd daemon offering unencrypted TCP/IP connections
-on localhost port 5000 and use the test driver with default
-settings.
-
-qemu+libssh2://user@host/system?known_hosts=/home/user/.ssh/known_hosts
-Connect to a remote host using a ssh connection with the libssh2 driver
-and use a different known_hosts file.
-qemu+libssh://user@host/system?known_hosts=/home/user/.ssh/known_hosts
-Connect to a remote host using a ssh connection with the libssh driver
-and use a different known_hosts file.
-
-
- Extra parameters
-
-
-Extra parameters can be added to remote URIs as part
-of the query string (the part following ?).
-Remote URIs understand the extra parameters shown below.
-Any others are passed unmodified through to the back end.
-Note that parameter values must be
-http://xmlsoft.org/html/libxml-uri.html#xmlURIEscapeStr;>URI-escaped.
-
-
-
- Name
- Transports
- Meaning
-
-
-
- name
-
-
- any transport
-
-
- The name passed to the remote virConnectOpen function. The
- name is normally formed by removing transport, hostname, port
- number, username and extra parameters from the remote URI, but in certain
- very complex cases it may be better to supply the name explicitly.
-
-
-
-
- Example: name=qemu:///system
-
-
-
- tls_priority
-
- tls
-
- A vaid GNUTLS priority string
-
-
-
-
- Example: tls_priority=NORMAL:-VERS-SSL3.0
-
-
-
- mode
-
- unix, ssh, libssh, libssh2
-
-
-autoautomatically determine the
daemon
-directconnect to per-driver daemons
-legacyconnect to libvirtd
-
- Can also be set in libvirt.conf as
remote_mode
-
-
-
-
- Example: mode=direct
-
-
-
- command
-
- ssh, ext
-
- The external command. For ext transport this is required.
- For ssh the default is ssh.
- The PATH is searched for the command.
-
-
-
-
- Example: command=/opt/openssh/bin/ssh
-
-
-
- socket
-
- unix, ssh, libssh2, libssh
-
- The path to the Unix domain socket, which overrides the
- compiled-in default. For ssh transport, this is passed to
- the remote netcat command (see next).
-
-
-
-
- Example:
socket=/opt/libvirt/run/libvirt/libvirt-sock
-
-
-
- netcat
-
- ssh, libssh2, libssh
-
- The name of the netcat command on the remote machine.
- The default is nc. For ssh transport, libvirt
- constructs an ssh command which looks like:
-
-command -p port [-l
[libvirt] [PATCH v2 03/15] docs: introduce rst2html as a mandatory tool for building docs
The rst2html tool is provided by python docutils, and as the name
suggests, it converts RST documents into HTML.
Basic rules are added for integrating RST docs into the website
build process.
This enables us to start writing docs on our website in RST format
instead of HTML, without changing the rest of our website templating
system away from XSLT yet.
Signed-off-by: Daniel P. Berrangé
---
docs/Makefile.am | 33 +++--
libvirt.spec.in | 2 ++
m4/virt-external-programs.m4 | 5 +
mingw-libvirt.spec.in| 1 +
4 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/docs/Makefile.am b/docs/Makefile.am
index 2a104bc837..7a8adbde2b 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -173,14 +173,26 @@ gif = \
internals_html_in = \
$(patsubst $(srcdir)/%,%,$(wildcard $(srcdir)/internals/*.html.in))
-internals_html = $(internals_html_in:%.html.in=%.html)
+kbase_rst = \
+ $(patsubst $(srcdir)/%,%,$(wildcard $(srcdir)/kbase/*.rst))
+kbase_rst_html_in = \
+ $(kbase_rst:%.rst=%.html.in)
+internals_html = \
+ $(internals_html_in:%.html.in=%.html) \
+ $(internals_rst_html_in:%.html.in=%.html)
internalsdir = $(HTML_DIR)/internals
internals_DATA = $(internals_html)
kbase_html_in = \
$(patsubst $(srcdir)/%,%,$(wildcard $(srcdir)/kbase/*.html.in))
-kbase_html = $(kbase_html_in:%.html.in=%.html)
+kbase_rst = \
+ $(patsubst $(srcdir)/%,%,$(wildcard $(srcdir)/kbase/*.rst))
+kbase_rst_html_in = \
+ $(kbase_rst:%.rst=%.html.in)
+kbase_html = \
+ $(kbase_html_in:%.html.in=%.html) \
+ $(kbase_rst_html_in:%.html.in=%.html)
kbasedir = $(HTML_DIR)/kbase
kbase_DATA = $(kbase_html)
@@ -191,9 +203,14 @@ dot_html_generated_in = \
news.html.in
dot_html_in = \
$(notdir $(wildcard $(srcdir)/*.html.in))
+dot_rst = \
+ $(notdir $(wildcard $(srcdir)/*.rst))
+dot_rst_html_in = \
+ $(dot_rst:%.rst=%.html)
dot_html = \
$(dot_html_generated_in:%.html.in=%.html) \
- $(dot_html_in:%.html.in=%.html)
+ $(dot_html_in:%.html.in=%.html) \
+ $(dot_rst_html_in:%.html.in=%.html)
htmldir = $(HTML_DIR)
html_DATA = $(css) $(png) $(gif) $(dot_html)
@@ -222,11 +239,11 @@ EXTRA_DIST= \
apibuild.py genaclperms.pl \
site.xsl subsite.xsl newapi.xsl page.xsl \
wrapstring.xsl \
- $(dot_html_in) $(gif) $(apipng) \
+ $(dot_html_in) $(dot_rst) $(gif) $(apipng) \
$(fig) $(png) $(css) \
$(javascript) $(logofiles) \
- $(internals_html_in) $(fonts) \
- $(kbase_html_in) \
+ $(internals_html_in) $(internals_rst) $(fonts) \
+ $(kbase_html_in) $(kbase_rst) \
aclperms.htmlinc \
hvsupport.pl \
$(schema_DATA)
@@ -281,6 +298,10 @@ EXTRA_DIST += \
%.png: %.fig
convert -rotate 90 $< $@
+%.html.in: %.rst
+ $(AM_V_GEN)$(MKDIR_P) `dirname $@` && \
+ $(RST2HTML) $< > $@
+
%.html.tmp: %.html.in site.xsl subsite.xsl page.xsl \
$(acl_generated)
$(AM_V_GEN)name=`echo $@ | sed -e 's/.tmp//'`; \
diff --git a/libvirt.spec.in b/libvirt.spec.in
index a6219da604..5c47dd3d7c 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -265,6 +265,8 @@ BuildRequires: automake
BuildRequires: gettext-devel
BuildRequires: libtool
BuildRequires: /usr/bin/pod2man
+# Replace with python3-docutils when we drop py2 support
+BuildRequires: /usr/bin/rst2html
%endif
BuildRequires: gcc
BuildRequires: git
diff --git a/m4/virt-external-programs.m4 b/m4/virt-external-programs.m4
index 0f995998c3..ed634a4c73 100644
--- a/m4/virt-external-programs.m4
+++ b/m4/virt-external-programs.m4
@@ -33,6 +33,11 @@ AC_DEFUN([LIBVIRT_CHECK_EXTERNAL_PROGRAMS], [
then
AC_MSG_ERROR("xsltproc is required to build libvirt")
fi
+ AC_PATH_PROGS([RST2HTML], [rst2html rst2html.py rst2html-3], [])
+ if test -z "$RST2HTML"
+ then
+AC_MSG_ERROR("rst2html is required to build libvirt")
+ fi
AC_PATH_PROG([AUGPARSE], [augparse], [/usr/bin/augparse])
AC_PROG_MKDIR_P
AC_PROG_LN_S
diff --git a/mingw-libvirt.spec.in b/mingw-libvirt.spec.in
index c29f3eeed2..35f1abc13d 100644
--- a/mingw-libvirt.spec.in
+++ b/mingw-libvirt.spec.in
@@ -82,6 +82,7 @@ BuildRequires: automake
BuildRequires: gettext-devel
BuildRequires: libtool
%endif
+BuildRequires: python3-docutils
BuildRequires: mingw32-libssh2
BuildRequires: mingw64-libssh2
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 00/15] docs: some refactoring and new docs about RPM deployment
This refactors existing docs related to the remote driver/daemon and
URIs. It then also adds a kbase page about RPM package options.
This introduces the use of RST for docs as a replacement for HTML.
The intent is that all new docs should use RST from this point.
The POD man pages will also be converted to RST. I'll post this as
a different series.
Note this series only touches the main HTML content, not the
styling / templating using XSL. Tackling that is a separate
job I've not attempted. My guess is that we'd use either Sphinx
or Pelican to do the templating.
Existing HTML docs are candidates for conversion to RST, however,
people should *NOT* attempt todo this manually.
In this series I've converted docs/kbase/ files, using the pandoc
tool. This does a pretty good job in general but does need some
manual cleanups. First I post-processed its output to change the
heading highlighting to follow the documented style
$ cat convert.pl
my @in = <>;
my @out;
for (my $i ; $i <= $#in; $i++) {
my $line = $in[$i];
if ($line =~ /^=+$/) {
my @newout = splice @out, 0, $i - 1;
push @newout, $line;
push @newout, $out[$i-1];
push @newout, $out[$i];
push @newout, $line;
push @newout, "\n";
push @newout, ".. contents::\n";
@out = @newout;
} elsif ($line =~ /^(-|~|^|')+$/) {
$line =~ s/-/=/g;
$line =~ s/~/-/g;
$line =~ s/\^/~/g;
$line =~ s/'/^/g;
push @out, $line;
} else {
push @out, $line;
}
}
print @out;
So I'm converting with
pandoc -f html -t rst < foo.html.in | perl convert.pl > foo.rst
$EDITOR foot.rst
git rm -f foot.html.in
git add foo.rst
After pandoc & the headings convert, there's usually manual fixes
needed to deal with a few oddities pandoc gets wrong, or looks
ugly. This is still way simpler than converting the doc to rst
manually.
Changed in v2:
- Added a style guide for RST docs
- Probe for rst2html's different names
- Added make rules for building rst
- Fixed permalinks in generated docs
- Misc CSS fixes
- Auto-converted all of kbase/ directory
Daniel P. Berrangé (15):
docs: split TLS certificate setup into its own file
docs: move docs about remote driver URIs into URI docs
docs: introduce rst2html as a mandatory tool for building docs
docs: adapt filling of section for rst2html output
docs: generate permalinks correctly for rst2html output
docs: relax CSS context match for pretty tables
docs: add styling for element
docs: add a minimal style guide for writing RST docs
docs: convert kbase/domainstatecapture.html.in to RST
docs: convert kbase/launch_security_sev.html.in to RST
docs: convert kbase/secureusage.html.in to RST
docs: convert kbase/locking.html.in to RST
docs: convert kbase/locking-lockd.html.in to RST
docs: convert kbase/locking-sanlock.html.in to RST
docs: add a kbase page about RPM packaging options
docs/Makefile.am | 33 +-
docs/aclpolkit.html.in | 20 +-
docs/docs.html.in | 6 +
docs/genaclperms.pl| 2 +-
docs/generic.css | 4 +-
docs/kbase.html.in | 4 +
docs/kbase/domainstatecapture.html.in | 303 ---
docs/kbase/domainstatecapture.rst | 255 +
docs/kbase/launch_security_sev.html.in | 533 ---
docs/kbase/launch_security_sev.rst | 529 +++
docs/kbase/locking-lockd.html.in | 160 --
docs/kbase/locking-lockd.rst | 121 +
docs/kbase/locking-sanlock.html.in | 247 -
docs/kbase/locking-sanlock.rst | 193 +++
docs/kbase/locking.html.in | 48 --
docs/kbase/locking.rst | 33 ++
docs/kbase/rpm-deployment.rst | 410 +++
docs/kbase/secureusage.html.in | 171 ---
docs/kbase/secureusage.rst | 131 +
docs/libvirt.css | 76 +--
docs/migration.html.in | 2 +-
docs/newapi.xsl| 4 +-
docs/page.xsl | 9 +-
docs/remote.html.in| 684 +
docs/styleguide.rst| 66 +++
docs/tlscerts.html.in | 413 +++
docs/uri.html.in | 263 --
libvirt.spec.in| 2 +
m4/virt-external-programs.m4 | 5 +
mingw-libvirt.spec.in | 1 +
30 files changed, 2478 insertions(+), 2250 deletions(-)
delete mode 100644 docs/kbase/domainstatecapture.html.in
create mode 100644 docs/kbase/domainstatecapture.rst
delete mode 100644 docs/kbase/launch_security_sev.html.in
create mode 100644 docs/kbase/launch_security_sev.rst
delete mode 100644 docs/kbase/locking-lockd.html.in
create mode 100644 docs/kbase/locking-lockd.rst
delete mode 100644 docs/kbase/locking-sanlock.html.in
Re: [libvirt] [PATCH v4 07/20] [ACKED] po: rewrite the way how we generate files
On Fri, Nov 08, 2019 at 04:42:08PM +0100, Pavel Hrdina wrote: > There was no need to handle files for translation from build directory > but that will change with following patches where we will stop > generating source files into source directory. > > In order to have them included for translation we have to prefix each > file with SRCDIR or BUILDDIR. > > Signed-off-by: Pavel Hrdina > Reviewed-by: Daniel P. Berrangé > --- > > Notes: > Changes in v2: > - add builddir paths for sc_po_check to detect generated source > files and generate correct diff if the check fails > > build-aux/syntax-check.mk | 8 +- > po/Makefile.am| 14 +- > po/POTFILES | 352 -- > po/POTFILES.in| 352 ++ > 4 files changed, 367 insertions(+), 359 deletions(-) > delete mode 100644 po/POTFILES > create mode 100644 po/POTFILES.in > diff --git a/po/Makefile.am b/po/Makefile.am > index 89e831f839..443d8a4dc1 100644 > --- a/po/Makefile.am > +++ b/po/Makefile.am > @@ -15,16 +15,21 @@ LANGS := \ > uk ur vi wba yo zh_CN zh_HK zh_TW zu > > > -POTFILE_DEPS := $(shell $(SED) 's,^,$(top_srcdir)/,' $(srcdir)/POTFILES) > +POTFILES_IN = $(srcdir)/POTFILES.in > +POTFILES: $(POTFILES_IN) > + $(AM_V_GEN) cat $(POTFILES_IN) | \ > + $(SED) 's|[@]SRCDIR[@]|$(top_srcdir)|' | \ > + $(SED) 's|[@]BUILDDIR[@]|$(top_builddir)|' > $@ > +POTFILE_DEPS = $(shell cat POTFILES) We've got a chicken & egg problem here. Previously POTFILES was committed to git, so it always existed. Now POTFILES.in is in git and POTFILES is generated. When you run make we get a message cat: POTFILES: No such file or directory Fortunately this is harmless AFAICT, make just continues with an empty POTFILES_DEPS variable and the lack of deps is not a problem since POTFILES doesn't exist yet. I wonder if we should try to fix it though ? Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/4] PCI hostdev partial assignment support
On 11/21/19 8:08 PM, Alex Williamson wrote: On Thu, 21 Nov 2019 19:19:13 -0300 Daniel Henrique Barboza wrote: [...] My question is: should Libvirt force function 0 to be present in boot time as well, regardless of whether the PPC64 guest or some cards are able to boot without it? In my reading of the PCI 3.0 spec, 3.2.2.3.4 indicates that multifunction devices are required to implement function 0 and that configuration software is under no obligation to scan for higher number functions if function 0 is not present and does not have the multifunction bit set in the header type register. With PCIe, where link information, payload size, ARI, VC, etc are exposed in config space, many of these are only valid or have specific means only for function 0. What you're seeing on PPC is, I think, more typical of paravirtualized PCI enumeration, where you're only seeing a view of the bus as allowed by a hypervisor. I can't say whether the pcie_scan_all boot option was added to allow discovery of devices as in your configuration or simply to correct cases where function 0 forgot to implement the multifunction Just checked. Neither the Power 8 host nor the guest is booting with the pcie_scan_all option. bit.Whether libvirt wants to prevent this is more of a support question, it seems spec compliant hardware should never do this, but not all hardware is spec compliant. Libvirt should never generate such a configuration on it's own, but I wouldn't necessarily object if it allows a user to shoot themselves in the foot. Thanks, I am not so thrilled about it after what you said. It seems that the PPC64 guest is behaving differently from all other archs in this case, and I'd rather make PPC64 equal to everyone else rather than allowing the PPC64 user to expect that non-compliant behavior is allowed. Thanks, DHB Alex -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/2] Simplify hash table data free callbacks
On 11/22/19 1:00 PM, Daniel P. Berrangé wrote: See the patch commit message for the interesting details. Daniel P. Berrangé (2): conf: stop using hash key when free'ing hash entries util: consolidate on one free callback for hash data src/conf/domain_addr.c | 4 +-- src/conf/nwfilter_params.c | 2 +- src/conf/virchrdev.c| 43 +++-- src/conf/virdomainmomentobjlist.c | 3 +- src/conf/virnetworkobj.c| 2 +- src/libxl/libxl_logger.c| 2 +- src/locking/lock_daemon.c | 3 +- src/nwfilter/nwfilter_dhcpsnoop.c | 4 +-- src/nwfilter/nwfilter_learnipaddr.c | 2 +- src/qemu/qemu_block.c | 3 +- src/qemu/qemu_conf.c| 4 +-- src/qemu/qemu_conf.h| 2 +- src/qemu/qemu_domain.c | 3 +- src/qemu/qemu_migration.c | 3 +- src/qemu/qemu_monitor.c | 5 ++-- src/qemu/qemu_monitor.h | 2 +- src/qemu/qemu_monitor_json.c| 2 +- src/util/vircgroup.c| 2 +- src/util/virhash.c | 28 +-- src/util/virhash.h | 16 ++- src/util/virjson.c | 3 +- src/util/virjson.h | 2 +- src/util/virkeyfile.c | 2 +- src/util/virlockspace.c | 2 +- src/util/virobject.c| 3 +- src/util/virobject.h| 3 +- src/util/virsystemd.c | 8 +++--- 27 files changed, 70 insertions(+), 88 deletions(-) Reviewed-by: Michal Privoznik Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] fix bug libvirt daemon deadlock when another force console break down an existed console client when this deadlock hanppened, libvirtd backtrace as follow, a typical ABBA deadloc
Thank you!
But it looks like libvirtd daemon stream module still has bug lead to
process segfault if applied this patch(if not, it will lead libvirtd
deadlock).I have got the cause, but it seems difficult to fix it with
present codes。
Bug reproduce:
1. add sleep(3) in daemonStreamFilter() so it is easily to get the point,
as follows
static int
daemonStreamFilter(virNetServerClientPtr client,
virNetMessagePtr msg,
void *opaque)
{
daemonClientStream *stream = opaque;
int ret = 0;
static int first = 1;
//if (first) {
//notify_force_console();
//VIR_ERROR("notify force console to break this client down!client:
%p", client);
//sleep(30);
//VIR_ERROR("sleep 10 finished");
//}
VIR_ERROR("stream: %p, filter_id: %d", stream, stream->filterID);
virObjectUnlock(client);
sleep(3);
virMutexLock(>priv->lock);
virObjectLock(client);
2. build the libvirtd with new code, then replace the libvirtd
3. use virsh console to open a vm's console tty, then just input many chars
in the console
4. open new terminal, then use virsh console --force to break the previous
client, then you'll got libvirtd segfault
it looks because when daemonStreamEvent() with para events
as VIR_STREAM_EVENT_ERROR is called, it will remove stream and free stream,
but daemonStreamFilter still waiting stream->priv->lock mutex, thus cause
invalid memory access
To fix this, I think the most effective and clean way is to drop
client->priv->lock elements, just use client object lock。But for current
code, it's not easy to do that。
also， a workaroud scheme is to add a big lock, but is ugly and low
efficient。
Any good ideas?
Michal Privoznik 于2019年11月19日周二 下午11:41写道：
> On 11/19/19 12:39 PM, LanceLiu wrote:
> > (gdb) thread 23
> > [Switching to thread 23 (Thread 0x7fbb54810700 (LWP 296966))]
> > (gdb) bt
> > at rpc/virnetserverclient.c:1503
> > client=client@entry=0x5641d20e20a0, msg=0x7fbb08003ab0,
> rerr=rerr@entry=0x7fbb5480f7b0, procedure=201,
> > type=type@entry=3, serial=9) at rpc/virnetserverprogram.c:173
> > msg=, rerr=rerr@entry=0x7fbb5480f7b0,
> procedure=, serial=)
> > at rpc/virnetserverprogram.c:222
> > opaque=opaque@entry=0x5641d20e20a0) at stream.c:246
> > force=force@entry=true) at conf/virchrdev.c:386
> > at qemu/qemu_driver.c:16188
> > st=st@entry=0x7fbb08008fc0, flags=1) at libvirt-domain.c:9363
> > rerr=0x7fbb5480fc10, msg=, client=0x5641d20e4e80) at
> remote_dispatch.h:8540
> > rerr=0x7fbb5480fc10, args=0x7fbb08005eb0, ret=) at
> remote_dispatch.h:8505
> > server=0x5641d20be420, prog=0x5641d20da580) at
> rpc/virnetserverprogram.c:437
> > msg=0x5641d20e9a20) at rpc/virnetserverprogram.c:307
> > srv=0x5641d20be420) at rpc/virnetserver.c:148
> > ---Type to continue, or q to quit---
> >
> > (gdb) thread 26
> > [Switching to thread 26 (Thread 0x7fbb87a378c0 (LWP 295636))]
> > (gdb) bt
> > at stream.c:286
> > at rpc/virnetserverclient.c:1247
> > at rpc/virnetserverclient.c:1457
> > at util/vireventpoll.c:508
> >
> > Signed-off-by: LanceLiu
> > ---
> > src/remote/remote_daemon_stream.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/remote/remote_daemon_stream.c
> b/src/remote/remote_daemon_stream.c
> > index 1f6e783..b27348f 100644
> > --- a/src/remote/remote_daemon_stream.c
> > +++ b/src/remote/remote_daemon_stream.c
> > @@ -284,14 +284,16 @@ daemonStreamEvent(virStreamPtr st, int events,
> void *opaque)
> >* -1 on fatal client error
> >*/
> > static int
> > -daemonStreamFilter(virNetServerClientPtr client ATTRIBUTE_UNUSED,
> > +daemonStreamFilter(virNetServerClientPtr client,
>
> This does not look rebased onto current git HEAD ;-) "Luckily", the
> problem you're fixing is still there.
>
> > virNetMessagePtr msg,
> > void *opaque)
> > {
> > daemonClientStream *stream = opaque;
> > int ret = 0;
> >
> > +virObjectUnlock(client);
> > virMutexLock(>priv->lock);
> > +virObjectLock(client);
> >
> > if (msg->header.type != VIR_NET_STREAM &&
> > msg->header.type != VIR_NET_STREAM_HOLE)
> >
>
> I've reworded the commit message, put a comment to
> virNetServerClientFilterFunc() typedef, ACKed and pushed.
>
> Congratulations on your first libvirt contribution!
>
> Michal
>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/2] util: consolidate on one free callback for hash data
This previous commit introduced a simpler free callback for
hash data with only 1 arg, the value to free:
commit 49288fac965f0ee23db45d83ae4ef3a9a71dafd0
Author: Peter Krempa
Date: Wed Oct 9 15:26:37 2019 +0200
util: hash: Add possibility to use simpler data free function in virHash
It missed two functions in the hash table code which need
to call the alternate data free function, virHashRemoveEntry
and virHashRemoveSet.
After the previous patch though, there is no code that
makes functional use of the 2nd key arg in the data
free function. There is merely one log message that can
be dropped.
We can thus purge the current virHashDataFree callback
entirely, and rename virHashDataFreeSimple to replace
it.
Signed-off-by: Daniel P. Berrangé
---
src/conf/domain_addr.c | 4 ++--
src/conf/nwfilter_params.c | 2 +-
src/conf/virchrdev.c| 4 ++--
src/conf/virdomainmomentobjlist.c | 3 +--
src/conf/virnetworkobj.c| 2 +-
src/libxl/libxl_logger.c| 2 +-
src/locking/lock_daemon.c | 3 +--
src/nwfilter/nwfilter_dhcpsnoop.c | 4 ++--
src/nwfilter/nwfilter_learnipaddr.c | 2 +-
src/qemu/qemu_block.c | 3 +--
src/qemu/qemu_conf.c| 4 ++--
src/qemu/qemu_conf.h| 2 +-
src/qemu/qemu_domain.c | 3 +--
src/qemu/qemu_migration.c | 3 +--
src/qemu/qemu_monitor.c | 5 ++---
src/qemu/qemu_monitor.h | 2 +-
src/qemu/qemu_monitor_json.c| 2 +-
src/util/vircgroup.c| 2 +-
src/util/virhash.c | 28 +++-
src/util/virhash.h | 16 +++-
src/util/virjson.c | 3 +--
src/util/virjson.h | 2 +-
src/util/virkeyfile.c | 2 +-
src/util/virlockspace.c | 2 +-
src/util/virobject.c| 3 +--
src/util/virobject.h| 3 +--
src/util/virsystemd.c | 8
27 files changed, 43 insertions(+), 76 deletions(-)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 74d561cd12..165bd70f2b 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1037,14 +1037,14 @@
virDomainPCIAddressSetExtensionAlloc(virDomainPCIAddressSetPtr addrs,
if (VIR_ALLOC(addrs->zpciIds) < 0)
return -1;
-if (!(addrs->zpciIds->uids = virHashCreateFull(10, NULL, NULL,
+if (!(addrs->zpciIds->uids = virHashCreateFull(10, NULL,
virZPCIAddrKeyCode,
virZPCIAddrKeyEqual,
virZPCIAddrKeyCopy,
virZPCIAddrKeyFree)))
goto error;
-if (!(addrs->zpciIds->fids = virHashCreateFull(10, NULL, NULL,
+if (!(addrs->zpciIds->fids = virHashCreateFull(10, NULL,
virZPCIAddrKeyCode,
virZPCIAddrKeyEqual,
virZPCIAddrKeyCopy,
diff --git a/src/conf/nwfilter_params.c b/src/conf/nwfilter_params.c
index 7608d4960e..b1a2c50f27 100644
--- a/src/conf/nwfilter_params.c
+++ b/src/conf/nwfilter_params.c
@@ -619,7 +619,7 @@ virNWFilterVarCombIterGetVarValue(virNWFilterVarCombIterPtr
ci,
}
static void
-hashDataFree(void *payload, const void *name G_GNUC_UNUSED)
+hashDataFree(void *payload)
{
virNWFilterVarValueFree(payload);
}
diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c
index d5c0fdbe99..d4ca3188c5 100644
--- a/src/conf/virchrdev.c
+++ b/src/conf/virchrdev.c
@@ -212,7 +212,7 @@ typedef struct {
*
* @data Opaque data, struct holding information about the device
*/
-static void virChrdevHashEntryFree(void *data, const void *key G_GNUC_UNUSED)
+static void virChrdevHashEntryFree(void *data)
{
virChrdevHashEntry *ent = data;
@@ -455,6 +455,6 @@ int virChrdevOpen(virChrdevsPtr devs,
VIR_FREE(cbdata->path);
VIR_FREE(cbdata);
virMutexUnlock(>lock);
-virChrdevHashEntryFree(ent, NULL);
+virChrdevHashEntryFree(ent);
return -1;
}
diff --git a/src/conf/virdomainmomentobjlist.c
b/src/conf/virdomainmomentobjlist.c
index ef702eb6aa..18dbd434fb 100644
--- a/src/conf/virdomainmomentobjlist.c
+++ b/src/conf/virdomainmomentobjlist.c
@@ -262,8 +262,7 @@ virDomainMomentAssignDef(virDomainMomentObjListPtr moments,
static void
-virDomainMomentObjListDataFree(void *payload,
- const void *name G_GNUC_UNUSED)
+virDomainMomentObjListDataFree(void *payload)
{
virDomainMomentObjPtr obj = payload;
diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index a0daaefb74..5daf4a8cb1 100644
--- a/src/conf/virnetworkobj.c
+++
[libvirt] [PATCH 0/2] Simplify hash table data free callbacks
See the patch commit message for the interesting details. Daniel P. Berrangé (2): conf: stop using hash key when free'ing hash entries util: consolidate on one free callback for hash data src/conf/domain_addr.c | 4 +-- src/conf/nwfilter_params.c | 2 +- src/conf/virchrdev.c| 43 +++-- src/conf/virdomainmomentobjlist.c | 3 +- src/conf/virnetworkobj.c| 2 +- src/libxl/libxl_logger.c| 2 +- src/locking/lock_daemon.c | 3 +- src/nwfilter/nwfilter_dhcpsnoop.c | 4 +-- src/nwfilter/nwfilter_learnipaddr.c | 2 +- src/qemu/qemu_block.c | 3 +- src/qemu/qemu_conf.c| 4 +-- src/qemu/qemu_conf.h| 2 +- src/qemu/qemu_domain.c | 3 +- src/qemu/qemu_migration.c | 3 +- src/qemu/qemu_monitor.c | 5 ++-- src/qemu/qemu_monitor.h | 2 +- src/qemu/qemu_monitor_json.c| 2 +- src/util/vircgroup.c| 2 +- src/util/virhash.c | 28 +-- src/util/virhash.h | 16 ++- src/util/virjson.c | 3 +- src/util/virjson.h | 2 +- src/util/virkeyfile.c | 2 +- src/util/virlockspace.c | 2 +- src/util/virobject.c| 3 +- src/util/virobject.h| 3 +- src/util/virsystemd.c | 8 +++--- 27 files changed, 70 insertions(+), 88 deletions(-) -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/2] conf: stop using hash key when free'ing hash entries
The virChrdevHashEntryFree method uses the hash 'key'
as the name of the logfile it has to remove. By storing
a struct as the value which contains the stream and
the dev path, we can avoid relying on the hash key
when free'ing entries.
Signed-off-by: Daniel P. Berrangé
---
src/conf/virchrdev.c | 43 +--
1 file changed, 29 insertions(+), 14 deletions(-)
diff --git a/src/conf/virchrdev.c b/src/conf/virchrdev.c
index 4b8f526d35..d5c0fdbe99 100644
--- a/src/conf/virchrdev.c
+++ b/src/conf/virchrdev.c
@@ -202,23 +202,30 @@ static void virChrdevLockFileRemove(const char *dev
G_GNUC_UNUSED)
}
#endif /* #ifdef VIR_CHRDEV_LOCK_FILE_PATH */
+typedef struct {
+char *dev;
+virStreamPtr st;
+} virChrdevHashEntry;
+
/**
* Frees an entry from the hash containing domain's active devices
*
* @data Opaque data, struct holding information about the device
- * @name Path of the device.
*/
-static void virChrdevHashEntryFree(void *data,
-const void *name)
+static void virChrdevHashEntryFree(void *data, const void *key G_GNUC_UNUSED)
{
-const char *dev = name;
-virStreamPtr st = data;
+virChrdevHashEntry *ent = data;
+
+if (!ent)
+return;
/* free stream reference */
-virObjectUnref(st);
+virObjectUnref(ent->st);
/* delete lock file */
-virChrdevLockFileRemove(dev);
+virChrdevLockFileRemove(ent->dev);
+
+g_free(ent);
}
/**
@@ -290,9 +297,9 @@ static int virChrdevFreeClearCallbacks(void *payload,
const void *name G_GNUC_UNUSED,
void *data G_GNUC_UNUSED)
{
-virStreamPtr st = payload;
+virChrdevHashEntry *ent = payload;
-virFDStreamSetInternalCloseCb(st, NULL, NULL, NULL);
+virFDStreamSetInternalCloseCb(ent->st, NULL, NULL, NULL);
return 0;
}
@@ -337,7 +344,7 @@ int virChrdevOpen(virChrdevsPtr devs,
bool force)
{
virChrdevStreamInfoPtr cbdata = NULL;
-virStreamPtr savedStream;
+virChrdevHashEntry *ent;
char *path;
int ret;
bool added = false;
@@ -363,7 +370,7 @@ int virChrdevOpen(virChrdevsPtr devs,
virMutexLock(>lock);
-if ((savedStream = virHashLookup(devs->hash, path))) {
+if ((ent = virHashLookup(devs->hash, path))) {
if (!force) {
/* entry found, device is busy */
virMutexUnlock(>lock);
@@ -376,8 +383,8 @@ int virChrdevOpen(virChrdevsPtr devs,
* same thread. We need to unregister the callback and abort the
* stream manually before we create a new device connection.
*/
- virFDStreamSetInternalCloseCb(savedStream, NULL, NULL, NULL);
- virStreamAbort(savedStream);
+ virFDStreamSetInternalCloseCb(ent->st, NULL, NULL, NULL);
+ virStreamAbort(ent->st);
virHashRemoveEntry(devs->hash, path);
/* continue adding a new stream connection */
}
@@ -398,8 +405,15 @@ int virChrdevOpen(virChrdevsPtr devs,
if (VIR_ALLOC(cbdata) < 0)
goto error;
-if (virHashAddEntry(devs->hash, path, st) < 0)
+if (VIR_ALLOC(ent) < 0)
+goto error;
+
+ent->st = st;
+ent->dev = g_strdup(path);
+
+if (virHashAddEntry(devs->hash, path, ent) < 0)
goto error;
+ent = NULL;
added = true;
cbdata->devs = devs;
@@ -441,5 +455,6 @@ int virChrdevOpen(virChrdevsPtr devs,
VIR_FREE(cbdata->path);
VIR_FREE(cbdata);
virMutexUnlock(>lock);
+virChrdevHashEntryFree(ent, NULL);
return -1;
}
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 8/8] qemu: enable blockdev support
On Thu, Nov 21, 2019 at 06:32:11PM +, Daniel P. Berrangé wrote: > On Mon, Nov 18, 2019 at 06:02:08PM +0100, Peter Krempa wrote: > > Now that all pieces are in place (hopefully) let's enable -blockdev. > > > > We base the capability on presence of the fix for 'auto-read-only' on > > files so that blockdev works properly, mandate that qemu supports > > explicit SCSI id strings to avoid ABI regression and that the fix for > > 'savevm' is present so that internal snapshots work. > > IIUC, once we enable this, we are fully committed to blockdev > hereafter I forgot to say Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH 2/2] qapi: deprecate implicit filters
> On 22 Nov 2019, at 09:41, Markus Armbruster wrote: > > Reviving this old thread, because I'd like to connect it to more recent > discussions. > > Christophe de Dinechin mailto:dinec...@redhat.com>> > writes: > >> Markus Armbruster writes: >> >>> Peter Krempa writes: >>> >> [...] From my experience users report non-fatal messages mostly only if it is spamming the system log. One of instances are very unlikely to be noticed. In my experience it's better to notify us in libvirt of such change and we will try our best to fix it. >>> >>> How to best alert the layers above QEMU was one of the topic of the KVM >>> Forum 2018 BoF on deprecating stuff. Minutes: >>> >>>Message-ID: <87mur0ls8o@dusky.pond.sub.org> >>>https://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg05828.html >>> >>> Relevant part: >>> >>> * We need to communicate "you're using something that is deprecated". >>> How? Right now, we print a deprecation message. Okay when humans use >>> QEMU directly in a shell. However, when QEMU sits at the bottom of a >>> software stack, the message will likely end up in a log file that is >>> effectively write-only. >>> >>> - The one way to get people read log files is crashing their >>>application. A command line option --future could make QEMU crash >>>right after printing a deprecation message. This could help with >>>finding use of deprecated features in a testing environment. >>> >>> - A less destructive way to grab people's attention is to make things >>>run really, really slow: have QEMU go to sleep for a while after >>>printing a deprecation message. >>> >>> - We can also pass the buck to the next layer up: emit a QMP event. >>> >>>Sadly, by the time the next layer connects to QMP, plenty of stuff >>>already happened. We'd have to buffer deprecation events somehow. >>> >>>What would libvirt do with such an event? Log it, taint the domain, >>>emit a (libvirt) event to pass it on to the next layer up. >>> >>> - A completely different idea is to have a configuratin linter. To >>>support doing this at the libvirt level, QEMU could expose "is >>>deprecated" in interface introspection. Feels feasible for QMP, >>>where we already have sufficiently expressive introspection. For >>>CLI, we'd first have to provide that (but we want that anyway). >>> >>> - We might also want to dispay deprecation messages in QEMU's GUI >>>somehow, or on serial consoles. >> >> Sorry for catching up late, this mail thread happened during my PTO. >> >> I remember bringing up at the time [1] that the correct solution needs >> to take into account usage models that vary from >> >> - a workstation case, where displaying an error box is easy and >> convenient, >> >> - to local headless VMs where system-level notification would do the job >> better, allowing us to leverage things like system-wide email notifications >> >> - to large-scale collections of VMs managed by some layered product, >> where the correct reporting would be through something like Insights, >> i.e. you don't scan individual logs, you want something like "913 VMs >> are using deprecated X" >> >> To me, that implies that we need to have a clear division of roles, with >> a standard way to >> >> a) produce the errors, >> b) propagate them, >> c) consume them (at least up to libvirt) >> >> Notice that this work has already been done for "real" errors, >> i.e. there is a real QAPI notion of "errors". AFAICT, warn_report does >> not connect to it, though, it goes through error_vprintf which is really >> just basic logging. >> >> So would it make sense to: >> >> 1. Add a deprecation_report() alongside warn_report()? > > "Grepability" alone would make that worthwhile, I think. > >> 2. Connect warn_report() and all the error_vprintf output to QAPI, >> e.g. using John's suggestion of adding the messages using some >> "warning" or "deprecated" tag? > > This is the difficult part. See my "Exposing feature deprecation to > machine clients (was: [Qemu-devel] [PATCH 2/2] qapi: deprecate implicit > filters)" in this thread. Quote: > >Propagating errors is painful. It has caused massive churn all over the >place. > >I don't think we can hitch deprecation info to the existing error >propagation, since we need to take the success path back to the QMP >core, not an error path. > >Propagating a second object for warnings... thanks, but no thanks. > >The QMP core could provide a function for recording deprecation info for >the currently executing QMP command. This is how we used to record >errors in QMP commands, until Anthony rammed through what we have now. >The commit messages (e.g. d5ec4f27c38) provide no justification. I >dimly recall adamant (oral?) claims that recording errors in the Monitor >object cannot work for us. > >I smell a swamp. This looks
Re: [libvirt] [PATCH 3/3] scripts: check-aclrules: use regular expressions less often
On Fri, Nov 22, 2019 at 11:37:31AM +0100, Ján Tomko wrote: > On Fri, Nov 22, 2019 at 09:00:12AM +0100, Erik Skultety wrote: > > On Wed, Nov 20, 2019 at 07:32:46PM +0100, Ján Tomko wrote: > > > Use a simple > > > if "substr" in line > > > before running a regular expression, which saves time, > > > especially if the regex has a capture group. > > > > > > This reduces runtime of the check by almost 78 % for me. > > > > > > Signed-off-by: Ján Tomko > > > --- > > > scripts/check-aclrules.py | 26 +++--- > > > 1 file changed, 19 insertions(+), 7 deletions(-) > > ... > > > > > # every func listed there, has an impl which calls > > > # an ACL function > > > if intable: > > > -assign = re.search(r'''\.(\w+)\s*=\s*(\w+),?''', line) > > > +assign = None > > > +if '"' in line: > > > > ^This doesn't quite work, does it? I don't see any '"' in the driver API > > mapping structures. The only reliable pattern I see we could use is ', /* ' > > but in order to use that, we'd have to unify a few source files, mainly > > vz_driver.c, doing: > > > > git grep -Enpi ", \s+/\* " | grep driver.c > > > > should help identifying them. > > Oops, that should've been "=". > > I have no idea how I managed to type that equal sign sideways. =) I thought so, but with the pattern I suggested, you can narrow down the final set of lines even more, so might turn out to be even faster than a plain "=" Erik -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] scripts: check-aclrules: use regular expressions less often
On Fri, Nov 22, 2019 at 09:00:12AM +0100, Erik Skultety wrote: On Wed, Nov 20, 2019 at 07:32:46PM +0100, Ján Tomko wrote: Use a simple if "substr" in line before running a regular expression, which saves time, especially if the regex has a capture group. This reduces runtime of the check by almost 78 % for me. Signed-off-by: Ján Tomko --- scripts/check-aclrules.py | 26 +++--- 1 file changed, 19 insertions(+), 7 deletions(-) ... # every func listed there, has an impl which calls # an ACL function if intable: -assign = re.search(r'''\.(\w+)\s*=\s*(\w+),?''', line) +assign = None +if '"' in line: ^This doesn't quite work, does it? I don't see any '"' in the driver API mapping structures. The only reliable pattern I see we could use is ', /* ' but in order to use that, we'd have to unify a few source files, mainly vz_driver.c, doing: git grep -Enpi ", \s+/\* " | grep driver.c should help identifying them. Oops, that should've been "=". I have no idea how I managed to type that equal sign sideways. =) Jano With this one addressed: Reviewed-by: Erik Skultety signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/4] qemu: Use host-model CPU on s390 by default
On 11/19/19 3:02 PM, Jiri Denemark wrote: On Mon, Nov 18, 2019 at 18:34:36 +0100, Boris Fiuczynski wrote: On 11/15/19 4:14 PM, Jiri Denemark wrote: On Fri, Nov 15, 2019 at 15:55:04 +0100, Christian Borntraeger wrote: On 15.11.19 15:47, Jiri Denemark wrote: On Fri, Nov 15, 2019 at 15:12:18 +0100, Boris Fiuczynski wrote: Just a heads up. After installing libvirt rpms of this branch all my existing kvm s390 domains ended up with qemu Newly defined domains without specified cpu do so as well. Unless the domains are all TCG, it seems your QEMU is too old. You need a fairly recent one which contains commit v4.1.0-1683-gde60a92ea7 (s390x/kvm: Set default cpu model for all machine classes) I the domains all use KVM and you have new enough QEMU, there might be a bug somewhere. Which should not happen :=) So shouldnt libvirt fence this rework (add default model) to qemu 4.2 and newer? Libvirt does all this only if query-machines returns default-cpu-type, which is introduced in 4.2. But since it was introduced earlier, anyone using qemu from git between the two commits will see this behavior. Somewhat similar thing will happen on ppc64, but even with the current master. Everything should be OK once QEMU 4.2.0 final release is used, though (since it will contain all required patches). Jirka I tested with a newer version of qemu and it worked as you outlined. After that I also tested with qemu v4.1.0. I was a bit surprised at first that a default host-model cpu was generated since I though it would only be done when the qemu has the commit your specified above. After reading your patch 4 the generation is tied to the cpu-model support in qemu. Since this became available on s390 with qemu v2.8.0 I created an additional test patch just to ensure that we do not lose backwards compatibility. Oh, are you saying QEMU on s390 returns default-cpu-type in query-machines reply even with v4.1.0 and older? I don't see it in our test replies from anything older then v4.2.0. Anyway, libvirt should not set the default CPU with QEMU 4.1.0 and if it does, I have a bug in my patches :-) No you don't have a bug in your patches. I had a bug in my tests. :-( It works as you outlined. The cpu-model is set if QEMU 4.2.0 is used but not for 4.1.0 and older. qemuDomainDefSetDefaultCPU should not be called at all on QEMU 4.1.0 or older. The check for cpu-model support in this function is an additional check and this support should not be sufficient for the default CPU to be filled in. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Matthias Hartmann Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC 30/40] backup: Introduce virDomainBackup APIs
On Fri, Nov 22, 2019 at 09:06:31AM +0100, Peter Krempa wrote:
> On Thu, Nov 21, 2019 at 18:54:12 +, Daniel Berrange wrote:
> > On Fri, Oct 18, 2019 at 06:11:15PM +0200, Peter Krempa wrote:
> > > From: Eric Blake
> > >
> > > Introduce a few new public APIs related to incremental backups. This
> > > builds on the previous notion of a checkpoint (without an existing
> > > checkpoint, the new API is a full backup, differing from
> > > virDomainBlockCopy in the point of time chosen and in operation on
> > > multiple disks at once); and also allows creation of a new checkpoint
> > > at the same time as starting the backup (after all, an incremental
> > > backup is only useful if it covers the state since the previous
> > > backup).
> > >
> > > A backup job also affects filtering a listing of domains, as well as
> > > adding event reporting for signaling when a push model backup
> > > completes (where the hypervisor creates the backup); note that the
> > > pull model does not have an event (starting the backup lets a third
> > > party access the data, and only the third party knows when it is
> > > finished).
> > >
> > > Since multiple backup jobs can be run in parallel in the future (well,
> > > qemu doesn't support it yet, but we don't want to preclude the idea),
> > > virDomainBackupBegin() returns a positive job id, and the id is also
> > > visible in the backup XML. But until a future libvirt release adds a
> > > bunch of APIs related to parallel job management where job ids will
> > > actually matter, the documentation is also clear that job id 0 means
> > > the 'currently running backup job' (provided one exists), for use in
> > > virDomainBackupGetXMLDesc() and virDomainBackupEnd().
> > >
> > > The full list of new APIs:
> > > virDomainBackupBegin;
> > > virDomainBackupEnd;
> > > virDomainBackupGetXMLDesc;
> > >
> > > Signed-off-by: Eric Blake
> > > Reviewed-by: Daniel P. Berrangé
> > > ---
> > > include/libvirt/libvirt-domain.h | 26 -
> > > src/driver-hypervisor.h | 20
> > > src/libvirt-domain-checkpoint.c | 7 +-
> > > src/libvirt-domain.c | 191 +++
> > > src/libvirt_public.syms | 8 ++
> > > tools/virsh-domain.c | 4 +-
> > > 6 files changed, 252 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/include/libvirt/libvirt-domain.h
> > > b/include/libvirt/libvirt-domain.h
> > > index 22277b0a84..2d9f69f7d4 100644
> > > --- a/include/libvirt/libvirt-domain.h
> > > +++ b/include/libvirt/libvirt-domain.h
> > > @@ -3267,6 +3267,7 @@ typedef enum {
> >
> >
> > >
> > > +/**
> > > + * VIR_DOMAIN_JOB_ID:
> > > + *
> > > + * virDomainGetJobStats field: the id of the job (so far, only for jobs
> > > + * started by virDomainBackupBegin()), as VIR_TYPED_PARAM_INT.
> > > + */
> > > +# define VIR_DOMAIN_JOB_ID "id"
> > > +
> > > /**
> > > * VIR_DOMAIN_JOB_TIME_ELAPSED:
> > > *
> > > @@ -4106,7 +4115,8 @@ typedef void
> > > (*virConnectDomainEventMigrationIterationCallback)(virConnectPtr co
> > > * @nparams: size of the params array
> > > * @opaque: application specific data
> > > *
> > > - * This callback occurs when a job (such as migration) running on the
> > > domain
> > > + * This callback occurs when a job (such as migration or push-model
> > > + * virDomainBackupBegin()) running on the domain
> > > * is completed. The params array will contain statistics of the just
> > > completed
> > > * job as virDomainGetJobStats would return. The callback must not free
> > > @params
> > > * (the array will be freed once the callback finishes).
> > > @@ -4916,4 +4926,18 @@ int virDomainGetGuestInfo(virDomainPtr domain,
> > >int *nparams,
> > >unsigned int flags);
> > >
> > > +
> > > +int virDomainBackupBegin(virDomainPtr domain,
> > > + const char *backupXML,
> > > + const char *checkpointXML,
> > > + unsigned int flags);
> > > +
> > > +char *virDomainBackupGetXMLDesc(virDomainPtr domain,
> > > +int id,
> > > +unsigned int flags);
> > > +
> > > +int virDomainBackupEnd(virDomainPtr domain,
> > > + int id,
> > > + unsigned int flags);
> >
> > So this is still using a plain integer job ID, which is a concern
> > wrt future extensibility.
>
> My current plan is to go ahead with API based on this old one but with
> no support for any parallel jobs. Basically the same thing but 'id'
> argument removed.
>
> This actually fits in with the original documentation which was already
> ACKed for the virDomainBackupBegin API which said that the backup job
> uses the domain async job infrastructure. This means that the
> virDomainAbortJob and virDomainGetJobStats can be used to monitor the
> blockjob. It also gives us the
Re: [libvirt] [PATCH] fix bug libvirt daemon deadlock when another force console break down an existed console client when this deadlock hanppened, libvirtd backtrace as follow, a typical ABBA deadloc
On 11/22/19 8:00 AM, Lance Liu wrote:
Thank you!
But it looks like libvirtd daemon stream module still has bug lead to
process segfault if applied this patch(if not, it will lead libvirtd
deadlock).I have got the cause, but it seems difficult to fix it with
present codes。
Bug reproduce:
1. add sleep(3) in daemonStreamFilter() so it is easily to get the
point, as follows
static int
daemonStreamFilter(virNetServerClientPtr client,
virNetMessagePtr msg,
void *opaque)
{
daemonClientStream *stream = opaque;
int ret = 0;
static int first = 1;
// if (first) {
// notify_force_console();
// VIR_ERROR("notify force console to break this client
down!client: %p", client);
// sleep(30);
// VIR_ERROR("sleep 10 finished");
// }
VIR_ERROR("stream: %p, filter_id: %d", stream, stream->filterID);
virObjectUnlock(client);
sleep(3);
virMutexLock(>priv->lock);
virObjectLock(client);
2. build the libvirtd with new code, then replace the libvirtd
3. use virsh console to open a vm's console tty, then just input many
chars in the console
4. open new terminal, then use virsh console --force to break the
previous client, then you'll got libvirtd segfault
it looks because when daemonStreamEvent() with para events
as VIR_STREAM_EVENT_ERROR is called, it will remove stream and free stream,
but daemonStreamFilter still waiting stream->priv->lock mutex, thus
cause invalid memory access
Yes, this is exactly what is happening.
To fix this, I think the most effective and clean way is to drop
client->priv->lock elements, just use client object lock。But for
current code, it's not easy to do that。
also， a workaroud scheme is to add a big lock, but is ugly and low
efficient。
Any good ideas?
I think we need to get back to drawing board. In the original problem
you observed ABBA, but also was patching older libvirt. I convinced
myself that the problem still occurs even with (at that point) current
master:
1) virNetServerClientDispatchEvent() locks @client,
virNetServerClientDispatchRead()
daemonStreamFilter() which locks stream->priv->lock;
on the other hand:
2) daemonStreamEvent() locks stream->priv->lock,
virNetServerClientClose() which locks @client; /* there are other
functions called over @client which also lock @client, I've just picked
one */
So there is ABBA pattern. However, there might be another lock that is
breaking the pattern (although I don't see it now): CAB and another
thread then does CBA.
Do you have a reproducer for your original problem? Does it reproduce on
the current master (after you revert the patch I've merged of course).
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/8] conf: domcaps: Add 'backingStoreInput' domain capability
On Fri, Nov 22, 2019 at 10:18:02AM +0100, Peter Krempa wrote: > On Thu, Nov 21, 2019 at 18:19:55 +, Daniel Berrange wrote: > > On Mon, Nov 18, 2019 at 06:02:01PM +0100, Peter Krempa wrote: > > > Historically we've only supported the as an output-only > > > element for domain disks. The documentation states that it may become > > > supported on input. To allow management apps detectin once that happens > > > > s/detectin/to detect/ > > > > > add a domain capability which will be asserted if the hypervisor driver > > > will be able to obey the as configured on input. > > > > > > Signed-off-by: Peter Krempa > > > --- > > > docs/formatdomaincaps.html.in | 7 +++ > > > docs/schemas/domaincaps.rng | 9 + > > > src/conf/domain_capabilities.c| 1 + > > > src/conf/domain_capabilities.h| 1 + > > > tests/domaincapsdata/libxl-xenfv.xml | 1 + > > > tests/domaincapsdata/libxl-xenpv.xml | 1 + > > > tests/domaincapsdata/qemu_1.5.3-q35.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_1.5.3-tcg.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_1.5.3.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_1.6.0-q35.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_1.6.0-tcg.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_1.6.0.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_1.7.0-q35.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_1.7.0-tcg.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_1.7.0.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_2.1.1-q35.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_2.1.1-tcg.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_2.1.1.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_2.10.0-q35.x86_64.xml | 1 + > > > tests/domaincapsdata/qemu_2.10.0-tcg.x86_64.xml | 1 + > > > tests/domaincapsdata/qemu_2.10.0-virt.aarch64.xml | 1 + > > > tests/domaincapsdata/qemu_2.10.0.aarch64.xml | 1 + > > > tests/domaincapsdata/qemu_2.10.0.ppc64.xml| 1 + > > > tests/domaincapsdata/qemu_2.10.0.s390x.xml| 1 + > > > tests/domaincapsdata/qemu_2.10.0.x86_64.xml | 1 + > > > tests/domaincapsdata/qemu_2.11.0-q35.x86_64.xml | 1 + > > > tests/domaincapsdata/qemu_2.11.0-tcg.x86_64.xml | 1 + > > > tests/domaincapsdata/qemu_2.11.0.s390x.xml| 1 + > > > tests/domaincapsdata/qemu_2.11.0.x86_64.xml | 1 + > > > tests/domaincapsdata/qemu_2.12.0-q35.x86_64.xml | 1 + > > > tests/domaincapsdata/qemu_2.12.0-tcg.x86_64.xml | 1 + > > > tests/domaincapsdata/qemu_2.12.0-virt.aarch64.xml | 1 + > > > tests/domaincapsdata/qemu_2.12.0.aarch64.xml | 1 + > > > tests/domaincapsdata/qemu_2.12.0.ppc64.xml| 1 + > > > tests/domaincapsdata/qemu_2.12.0.s390x.xml| 1 + > > > tests/domaincapsdata/qemu_2.12.0.x86_64.xml | 1 + > > > tests/domaincapsdata/qemu_2.4.0-q35.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_2.4.0-tcg.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_2.4.0.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_2.5.0-q35.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_2.5.0-tcg.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_2.5.0.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_2.6.0-q35.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_2.6.0-tcg.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_2.6.0-virt.aarch64.xml | 1 + > > > tests/domaincapsdata/qemu_2.6.0.aarch64.xml | 1 + > > > tests/domaincapsdata/qemu_2.6.0.ppc64.xml | 1 + > > > tests/domaincapsdata/qemu_2.6.0.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_2.7.0-q35.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_2.7.0-tcg.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_2.7.0.s390x.xml | 1 + > > > tests/domaincapsdata/qemu_2.7.0.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_2.8.0-q35.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_2.8.0-tcg.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_2.8.0.s390x.xml | 1 + > > > tests/domaincapsdata/qemu_2.8.0.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_2.9.0-q35.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_2.9.0-tcg.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_2.9.0.ppc64.xml | 1 + > > > tests/domaincapsdata/qemu_2.9.0.s390x.xml | 1 + > > > tests/domaincapsdata/qemu_2.9.0.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_3.0.0-q35.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_3.0.0-tcg.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_3.0.0.ppc64.xml | 1 + > > > tests/domaincapsdata/qemu_3.0.0.s390x.xml | 1 + > > > tests/domaincapsdata/qemu_3.0.0.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_3.1.0-q35.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_3.1.0-tcg.x86_64.xml| 1 + > > > tests/domaincapsdata/qemu_3.1.0.ppc64.xml | 1 + > > >
Re: [libvirt] [PATCH 1/8] conf: domcaps: Add 'backingStoreInput' domain capability
On Thu, Nov 21, 2019 at 18:19:55 +, Daniel Berrange wrote: > On Mon, Nov 18, 2019 at 06:02:01PM +0100, Peter Krempa wrote: > > Historically we've only supported the as an output-only > > element for domain disks. The documentation states that it may become > > supported on input. To allow management apps detectin once that happens > > s/detectin/to detect/ > > > add a domain capability which will be asserted if the hypervisor driver > > will be able to obey the as configured on input. > > > > Signed-off-by: Peter Krempa > > --- > > docs/formatdomaincaps.html.in | 7 +++ > > docs/schemas/domaincaps.rng | 9 + > > src/conf/domain_capabilities.c| 1 + > > src/conf/domain_capabilities.h| 1 + > > tests/domaincapsdata/libxl-xenfv.xml | 1 + > > tests/domaincapsdata/libxl-xenpv.xml | 1 + > > tests/domaincapsdata/qemu_1.5.3-q35.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_1.5.3-tcg.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_1.5.3.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_1.6.0-q35.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_1.6.0-tcg.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_1.6.0.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_1.7.0-q35.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_1.7.0-tcg.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_1.7.0.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.1.1-q35.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.1.1-tcg.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.1.1.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.10.0-q35.x86_64.xml | 1 + > > tests/domaincapsdata/qemu_2.10.0-tcg.x86_64.xml | 1 + > > tests/domaincapsdata/qemu_2.10.0-virt.aarch64.xml | 1 + > > tests/domaincapsdata/qemu_2.10.0.aarch64.xml | 1 + > > tests/domaincapsdata/qemu_2.10.0.ppc64.xml| 1 + > > tests/domaincapsdata/qemu_2.10.0.s390x.xml| 1 + > > tests/domaincapsdata/qemu_2.10.0.x86_64.xml | 1 + > > tests/domaincapsdata/qemu_2.11.0-q35.x86_64.xml | 1 + > > tests/domaincapsdata/qemu_2.11.0-tcg.x86_64.xml | 1 + > > tests/domaincapsdata/qemu_2.11.0.s390x.xml| 1 + > > tests/domaincapsdata/qemu_2.11.0.x86_64.xml | 1 + > > tests/domaincapsdata/qemu_2.12.0-q35.x86_64.xml | 1 + > > tests/domaincapsdata/qemu_2.12.0-tcg.x86_64.xml | 1 + > > tests/domaincapsdata/qemu_2.12.0-virt.aarch64.xml | 1 + > > tests/domaincapsdata/qemu_2.12.0.aarch64.xml | 1 + > > tests/domaincapsdata/qemu_2.12.0.ppc64.xml| 1 + > > tests/domaincapsdata/qemu_2.12.0.s390x.xml| 1 + > > tests/domaincapsdata/qemu_2.12.0.x86_64.xml | 1 + > > tests/domaincapsdata/qemu_2.4.0-q35.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.4.0-tcg.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.4.0.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.5.0-q35.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.5.0-tcg.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.5.0.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.6.0-q35.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.6.0-tcg.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.6.0-virt.aarch64.xml | 1 + > > tests/domaincapsdata/qemu_2.6.0.aarch64.xml | 1 + > > tests/domaincapsdata/qemu_2.6.0.ppc64.xml | 1 + > > tests/domaincapsdata/qemu_2.6.0.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.7.0-q35.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.7.0-tcg.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.7.0.s390x.xml | 1 + > > tests/domaincapsdata/qemu_2.7.0.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.8.0-q35.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.8.0-tcg.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.8.0.s390x.xml | 1 + > > tests/domaincapsdata/qemu_2.8.0.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.9.0-q35.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.9.0-tcg.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_2.9.0.ppc64.xml | 1 + > > tests/domaincapsdata/qemu_2.9.0.s390x.xml | 1 + > > tests/domaincapsdata/qemu_2.9.0.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_3.0.0-q35.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_3.0.0-tcg.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_3.0.0.ppc64.xml | 1 + > > tests/domaincapsdata/qemu_3.0.0.s390x.xml | 1 + > > tests/domaincapsdata/qemu_3.0.0.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_3.1.0-q35.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_3.1.0-tcg.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_3.1.0.ppc64.xml | 1 + > > tests/domaincapsdata/qemu_3.1.0.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_4.0.0-q35.x86_64.xml| 1 + > > tests/domaincapsdata/qemu_4.0.0-tcg.x86_64.xml| 1 + > >
Re: [libvirt] [Qemu-devel] [PATCH 2/2] qapi: deprecate implicit filters
Reviving this old thread, because I'd like to connect it to more recent discussions. Christophe de Dinechin writes: > Markus Armbruster writes: > >> Peter Krempa writes: >> > [...] >>> From my experience users report non-fatal messages mostly only if it is >>> spamming the system log. One of instances are very unlikely to be >>> noticed. >>> >>> In my experience it's better to notify us in libvirt of such change and >>> we will try our best to fix it. >> >> How to best alert the layers above QEMU was one of the topic of the KVM >> Forum 2018 BoF on deprecating stuff. Minutes: >> >> Message-ID: <87mur0ls8o@dusky.pond.sub.org> >> https://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg05828.html >> >> Relevant part: >> >> * We need to communicate "you're using something that is deprecated". >> How? Right now, we print a deprecation message. Okay when humans use >> QEMU directly in a shell. However, when QEMU sits at the bottom of a >> software stack, the message will likely end up in a log file that is >> effectively write-only. >> >> - The one way to get people read log files is crashing their >> application. A command line option --future could make QEMU crash >> right after printing a deprecation message. This could help with >> finding use of deprecated features in a testing environment. >> >> - A less destructive way to grab people's attention is to make things >> run really, really slow: have QEMU go to sleep for a while after >> printing a deprecation message. >> >> - We can also pass the buck to the next layer up: emit a QMP event. >> >> Sadly, by the time the next layer connects to QMP, plenty of stuff >> already happened. We'd have to buffer deprecation events somehow. >> >> What would libvirt do with such an event? Log it, taint the domain, >> emit a (libvirt) event to pass it on to the next layer up. >> >> - A completely different idea is to have a configuratin linter. To >> support doing this at the libvirt level, QEMU could expose "is >> deprecated" in interface introspection. Feels feasible for QMP, >> where we already have sufficiently expressive introspection. For >> CLI, we'd first have to provide that (but we want that anyway). >> >> - We might also want to dispay deprecation messages in QEMU's GUI >> somehow, or on serial consoles. > > Sorry for catching up late, this mail thread happened during my PTO. > > I remember bringing up at the time [1] that the correct solution needs > to take into account usage models that vary from > > - a workstation case, where displaying an error box is easy and > convenient, > > - to local headless VMs where system-level notification would do the job > better, allowing us to leverage things like system-wide email notifications > > - to large-scale collections of VMs managed by some layered product, > where the correct reporting would be through something like Insights, > i.e. you don't scan individual logs, you want something like "913 VMs > are using deprecated X" > > To me, that implies that we need to have a clear division of roles, with > a standard way to > > a) produce the errors, > b) propagate them, > c) consume them (at least up to libvirt) > > Notice that this work has already been done for "real" errors, > i.e. there is a real QAPI notion of "errors". AFAICT, warn_report does > not connect to it, though, it goes through error_vprintf which is really > just basic logging. > > So would it make sense to: > > 1. Add a deprecation_report() alongside warn_report()? "Grepability" alone would make that worthwhile, I think. > 2. Connect warn_report() and all the error_vprintf output to QAPI, >e.g. using John's suggestion of adding the messages using some >"warning" or "deprecated" tag? This is the difficult part. See my "Exposing feature deprecation to machine clients (was: [Qemu-devel] [PATCH 2/2] qapi: deprecate implicit filters)" in this thread. Quote: Propagating errors is painful. It has caused massive churn all over the place. I don't think we can hitch deprecation info to the existing error propagation, since we need to take the success path back to the QMP core, not an error path. Propagating a second object for warnings... thanks, but no thanks. The QMP core could provide a function for recording deprecation info for the currently executing QMP command. This is how we used to record errors in QMP commands, until Anthony rammed through what we have now. The commit messages (e.g. d5ec4f27c38) provide no justification. I dimly recall adamant (oral?) claims that recording errors in the Monitor object cannot work for us. I smell a swamp. Can we avoid plumbing deprecation info from command code to QMP core? Only if the QMP core itself can recognize deprecated interfaces. I believe it can for the cases we can expose in introspecion. Let me explain.
Re: [libvirt] [PATCH 8/8] qemu: enable blockdev support
On Thu, Nov 21, 2019 at 18:32:11 +, Daniel Berrange wrote: > On Mon, Nov 18, 2019 at 06:02:08PM +0100, Peter Krempa wrote: > > Now that all pieces are in place (hopefully) let's enable -blockdev. > > > > We base the capability on presence of the fix for 'auto-read-only' on > > files so that blockdev works properly, mandate that qemu supports > > explicit SCSI id strings to avoid ABI regression and that the fix for > > 'savevm' is present so that internal snapshots work. > > IIUC, once we enable this, we are fully committed to blockdev > hereafter > > > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > > index 6f23400f95..b5fa0fba7e 100644 > > --- a/src/qemu/qemu_capabilities.c > > +++ b/src/qemu/qemu_capabilities.c > > @@ -4476,13 +4476,15 @@ virQEMUCapsInitProcessCaps(virQEMUCapsPtr qemuCaps) > > virQEMUCapsClear(qemuCaps, QEMU_CAPS_DEVICE_VFIO_CCW); > > } > > > > -/* To avoid guest ABI regression, blockdev shall be enabled only when > > - * we are able to pass the custom 'device_id' for SCSI disks and > > cdroms. */ > > -if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_DEVICE_ID)) > > -virQEMUCapsClear(qemuCaps, QEMU_CAPS_BLOCKDEV); > > - > > if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CPU_UNAVAILABLE_FEATURES)) > > virQEMUCapsSet(qemuCaps, QEMU_CAPS_CANONICAL_CPU_FEATURES); > > + > > +/* To avoid guest ABI regression, blockdev shall be enabled only when > > + * we are able to pass the custom 'device_id' for SCSI disks and > > cdroms. */ > > +if (virQEMUCapsGet(qemuCaps, > > QEMU_CAPS_BLOCK_FILE_AUTO_READONLY_DYNAMIC) && > > +virQEMUCapsGet(qemuCaps, QEMU_CAPS_SCSI_DISK_DEVICE_ID) && > > +virQEMUCapsGet(qemuCaps, QEMU_CAPS_SAVEVM_MONITOR_NODES)) > > we must *not* add more capabilities in future to this list of three > that we're checking, as that would cause us to regress in use of blockdev I think we have plenty qemucapabilitiestests and 'latest' xml2argv tests which would catch us turning it off. I can possibly add as follow up a bunch of the test files based on 4.2 capabilities so that we have historical reference if the requirements would change. Luckily blockdev would not cause an ABI regression and theoretically we could enable it e.g. during migration as well. But I chose to go the trditional way. > > > +virQEMUCapsSet(qemuCaps, QEMU_CAPS_BLOCKDEV); > > I don't think that's a problem really - we'll just treat anything else we > find as a normal bug & make an effort to fix it as possible. We dont want > to continually turn off blockdev over & over every time we find a new bug. Definitely. This is a known set of qemu bugs which need to be fixed so that libvirt can use it. As of libvirt's implementation, all features which we supported with pre-blockdev should [*] work on blockdev as well. There is only one caveat and that is if a VM has a SD card as disk, blockdev is disabled for such a VM (sd-cards can't be hotplugged). This is because some non-x86 boards in qemu have sd-card which can't be instantiated via -device. Also qemu's documentation is terrible in this regard so I didn't pursue fixing this yet. [*] I tried my best to test as much as possible and I also got some help from Red Hat's QE and a few colleagues using it prior to being enabled in doing so. I'm very thankful for this as I identified a handful of corner cases which weren't treated properly. signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC 30/40] backup: Introduce virDomainBackup APIs
On Thu, Nov 21, 2019 at 18:54:12 +, Daniel Berrange wrote:
> On Fri, Oct 18, 2019 at 06:11:15PM +0200, Peter Krempa wrote:
> > From: Eric Blake
> >
> > Introduce a few new public APIs related to incremental backups. This
> > builds on the previous notion of a checkpoint (without an existing
> > checkpoint, the new API is a full backup, differing from
> > virDomainBlockCopy in the point of time chosen and in operation on
> > multiple disks at once); and also allows creation of a new checkpoint
> > at the same time as starting the backup (after all, an incremental
> > backup is only useful if it covers the state since the previous
> > backup).
> >
> > A backup job also affects filtering a listing of domains, as well as
> > adding event reporting for signaling when a push model backup
> > completes (where the hypervisor creates the backup); note that the
> > pull model does not have an event (starting the backup lets a third
> > party access the data, and only the third party knows when it is
> > finished).
> >
> > Since multiple backup jobs can be run in parallel in the future (well,
> > qemu doesn't support it yet, but we don't want to preclude the idea),
> > virDomainBackupBegin() returns a positive job id, and the id is also
> > visible in the backup XML. But until a future libvirt release adds a
> > bunch of APIs related to parallel job management where job ids will
> > actually matter, the documentation is also clear that job id 0 means
> > the 'currently running backup job' (provided one exists), for use in
> > virDomainBackupGetXMLDesc() and virDomainBackupEnd().
> >
> > The full list of new APIs:
> > virDomainBackupBegin;
> > virDomainBackupEnd;
> > virDomainBackupGetXMLDesc;
> >
> > Signed-off-by: Eric Blake
> > Reviewed-by: Daniel P. Berrangé
> > ---
> > include/libvirt/libvirt-domain.h | 26 -
> > src/driver-hypervisor.h | 20
> > src/libvirt-domain-checkpoint.c | 7 +-
> > src/libvirt-domain.c | 191 +++
> > src/libvirt_public.syms | 8 ++
> > tools/virsh-domain.c | 4 +-
> > 6 files changed, 252 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/libvirt/libvirt-domain.h
> > b/include/libvirt/libvirt-domain.h
> > index 22277b0a84..2d9f69f7d4 100644
> > --- a/include/libvirt/libvirt-domain.h
> > +++ b/include/libvirt/libvirt-domain.h
> > @@ -3267,6 +3267,7 @@ typedef enum {
>
>
> >
> > +/**
> > + * VIR_DOMAIN_JOB_ID:
> > + *
> > + * virDomainGetJobStats field: the id of the job (so far, only for jobs
> > + * started by virDomainBackupBegin()), as VIR_TYPED_PARAM_INT.
> > + */
> > +# define VIR_DOMAIN_JOB_ID "id"
> > +
> > /**
> > * VIR_DOMAIN_JOB_TIME_ELAPSED:
> > *
> > @@ -4106,7 +4115,8 @@ typedef void
> > (*virConnectDomainEventMigrationIterationCallback)(virConnectPtr co
> > * @nparams: size of the params array
> > * @opaque: application specific data
> > *
> > - * This callback occurs when a job (such as migration) running on the
> > domain
> > + * This callback occurs when a job (such as migration or push-model
> > + * virDomainBackupBegin()) running on the domain
> > * is completed. The params array will contain statistics of the just
> > completed
> > * job as virDomainGetJobStats would return. The callback must not free
> > @params
> > * (the array will be freed once the callback finishes).
> > @@ -4916,4 +4926,18 @@ int virDomainGetGuestInfo(virDomainPtr domain,
> >int *nparams,
> >unsigned int flags);
> >
> > +
> > +int virDomainBackupBegin(virDomainPtr domain,
> > + const char *backupXML,
> > + const char *checkpointXML,
> > + unsigned int flags);
> > +
> > +char *virDomainBackupGetXMLDesc(virDomainPtr domain,
> > +int id,
> > +unsigned int flags);
> > +
> > +int virDomainBackupEnd(virDomainPtr domain,
> > + int id,
> > + unsigned int flags);
>
> So this is still using a plain integer job ID, which is a concern
> wrt future extensibility.
My current plan is to go ahead with API based on this old one but with
no support for any parallel jobs. Basically the same thing but 'id'
argument removed.
This actually fits in with the original documentation which was already
ACKed for the virDomainBackupBegin API which said that the backup job
uses the domain async job infrastructure. This means that the
virDomainAbortJob and virDomainGetJobStats can be used to monitor the
blockjob. It also gives us the possibility to query the state of a
finised job by passing VIR_DOMAIN_JOB_STATS_COMPLETED to
virDomainGetJobStats. We also have an async event for reporting the job
state. I'll also consider removing virDomainBackupEnd for this
implementation as virDomainAbortJob should be enough.
I
Re: [libvirt] [PATCH 4/3] scripts: use in even more
On Wed, Nov 20, 2019 at 08:16:27PM +0100, Ján Tomko wrote:
> Signed-off-by: Ján Tomko
> ---
> scripts/check-aclrules.py| 2 +-
> scripts/check-driverimpls.py | 2 +-
> scripts/check-symfile.py | 6 +++---
> scripts/dtrace2systemtap.py | 4 ++--
> scripts/genpolkit.py | 4 ++--
> scripts/gensystemtap.py | 2 +-
> 6 files changed, 10 insertions(+), 10 deletions(-)
>
With the following squashed in:
diff --git a/scripts/dtrace2systemtap.py b/scripts/dtrace2systemtap.py
index cf4d8e1a79..d6bf1f8d1f 100755
--- a/scripts/dtrace2systemtap.py
+++ b/scripts/dtrace2systemtap.py
@@ -126,7 +126,7 @@ for file in filelist:
for idx in range(len(argbits)):
arg = argbits[idx]
isstr = False
-if arg.find("char *") != -1:
+if 'char *' in arg:
isstr = True
m = re.search(r'''^.*\s\*?(\S+)$''', arg)
Reviewed-by: Erik Skultety
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] scripts: check-aclrules: use regular expressions less often
On Wed, Nov 20, 2019 at 07:32:46PM +0100, Ján Tomko wrote: > Use a simple > if "substr" in line > before running a regular expression, which saves time, > especially if the regex has a capture group. > > This reduces runtime of the check by almost 78 % for me. > > Signed-off-by: Ján Tomko > --- > scripts/check-aclrules.py | 26 +++--- > 1 file changed, 19 insertions(+), 7 deletions(-) ... > # every func listed there, has an impl which calls > # an ACL function > if intable: > -assign = re.search(r'''\.(\w+)\s*=\s*(\w+),?''', line) > +assign = None > +if '"' in line: ^This doesn't quite work, does it? I don't see any '"' in the driver API mapping structures. The only reliable pattern I see we could use is ', /* ' but in order to use that, we'd have to unify a few source files, mainly vz_driver.c, doing: git grep -Enpi ", \s+/\* " | grep driver.c should help identifying them. With this one addressed: Reviewed-by: Erik Skultety -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
