Re: [libvirt] [PATCH v2 4/8] qemu: vfio-ccw device address generation
On 05/14/2018 06:30 PM, John Ferlan wrote: On 05/14/2018 03:43 AM, Boris Fiuczynski wrote: On 05/10/2018 10:52 PM, John Ferlan wrote: On 05/07/2018 10:41 AM, Boris Fiuczynski wrote: From: Shalini Chellathurai SarojaIntroduces the vfio-ccw model for mediated devices and prime vfio-ccw devices such that CCW address will be generated. Signed-off-by: Shalini Chellathurai Saroja Reviewed-by: Bjoern Walk Reviewed-by: Boris Fiuczynski Reviewed-by: Marc Hartmayer Reviewed-by: Stefan Zimmermann --- docs/schemas/domaincommon.rng | 5 - src/qemu/qemu_domain_address.c | 20 src/util/virmdev.c | 3 ++- src/util/virmdev.h | 1 + 4 files changed, 27 insertions(+), 2 deletions(-) Looking at all places using VIR_MDEV_MODEL_TYPE_VFIO_PCI - should this patch make a change to virDomainHostdevDefPostParse to do something similar - that is: if (model == VIR_MDEV_MODEL_TYPE_VFIO_CCW && dev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { ... error message } ? Let me know... I think it should and can add it before pushing... You are correct. Good catch! How about fixing it like this? if ((model == VIR_MDEV_MODEL_TYPE_VFIO_PCI && dev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) || (model == VIR_MDEV_MODEL_TYPE_VFIO_CCW && dev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) { virReportError(VIR_ERR_XML_ERROR, _("Unsupported address type '%s' with mediated " "device model '%s'"), virDomainDeviceAddressTypeToString(dev->info->type), virMediatedDeviceModelTypeToString(model)); return -1; } OK - added that... Besides that I just saw that the indention of the second parameter of method qemuDomainPrimeVfioDeviceAddresses is off by three blanks. ah true - adjusted that. I've merged with the latest top of tree and have pushed... So much flux in the capabilities lately... John You are correct that lots of changes happened in the capabilities. Thanks for merging the changes and pushing the series! -- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz 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 v2] xenconfig: xm: Fix checking for extra in parser
On 05/12/2018 09:45 AM, Filip Alac wrote: Parser assumed extra was always present when root was specified. Fixed by handling root and extra separately. Signed-off-by: Filip AlacReviewed-by: Jim Fehlig and pushed. Thanks! Regards, Jim --- src/xenconfig/xen_xm.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/xenconfig/xen_xm.c b/src/xenconfig/xen_xm.c index 8ef68bbc..4becb40b 100644 --- a/src/xenconfig/xen_xm.c +++ b/src/xenconfig/xen_xm.c @@ -91,10 +91,13 @@ xenParseXMOS(virConfPtr conf, virDomainDefPtr def) if (xenConfigGetString(conf, "root", , NULL) < 0) return -1; -if (root) { +if (root && extra) { if (virAsprintf(>os.cmdline, "root=%s %s", root, extra) < 0) return -1; -} else { +} else if (root) { +if (virAsprintf(>os.cmdline, "root=%s", root) < 0) +return -1; +} else if (extra) { if (VIR_STRDUP(def->os.cmdline, extra) < 0) return -1; } -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] libxl: don't set hasManagedSave when performing save
libxlDoDomainSave() is used in both the save and managedsave code paths but was unconditionally setting hasManagedSave to true on success. As a result, undefine would fail after a non-managed save/restore operation. E.g. virsh define; virsh start virsh save; virsh restore virsh shutdown virsh undefine error: Refusing to undefine while domain managed save image exists Modify libxlDoDomainSave() to take an additional parameter to specify managed vs non-managed save, and change callers to use it. Signed-off-by: Jim Fehlig--- src/libxl/libxl_driver.c | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 34adef8d48..df53dead0a 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -1653,8 +1653,10 @@ libxlDomainGetState(virDomainPtr dom, * virDomainObjPtr must be locked on invocation */ static int -libxlDoDomainSave(libxlDriverPrivatePtr driver, virDomainObjPtr vm, - const char *to) +libxlDoDomainSave(libxlDriverPrivatePtr driver, + virDomainObjPtr vm, + const char *to, + bool managed) { libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); libxlSavefileHeader hdr; @@ -1725,7 +1727,7 @@ libxlDoDomainSave(libxlDriverPrivatePtr driver, virDomainObjPtr vm, } libxlDomainCleanup(driver, vm); -vm->hasManagedSave = true; +vm->hasManagedSave = managed; ret = 0; cleanup: @@ -1772,7 +1774,7 @@ libxlDomainSaveFlags(virDomainPtr dom, const char *to, const char *dxml, if (virDomainObjCheckActive(vm) < 0) goto endjob; -if (libxlDoDomainSave(driver, vm, to) < 0) +if (libxlDoDomainSave(driver, vm, to, false) < 0) goto endjob; if (!vm->persistent) @@ -1989,7 +1991,7 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int flags) VIR_INFO("Saving state to %s", name); -if (libxlDoDomainSave(driver, vm, name) < 0) +if (libxlDoDomainSave(driver, vm, name, true) < 0) goto endjob; if (!vm->persistent) -- 2.16.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 12/13] util: storage: Store PR manager alias in the definition
On 05/14/2018 11:19 AM, Michal Privoznik wrote: > On 05/14/2018 12:45 PM, Peter Krempa wrote: >> Rather than always re-generating the alias store it in the definition >> and in the status XML. >> >> Signed-off-by: Peter Krempa>> --- >> src/qemu/qemu_command.c | 23 +++-- >> src/qemu/qemu_command.h | 3 +-- >> src/qemu/qemu_domain.c| 16 +-- >> src/qemu/qemu_hotplug.c | 34 >> ++- >> src/util/virstoragefile.c | 1 + >> src/util/virstoragefile.h | 3 +++ >> tests/qemustatusxml2xmldata/modern-in.xml | 4 >> 7 files changed, 37 insertions(+), 47 deletions(-) > > Yes, this makes sense to me. I've kept alias in status XML for all the > versions until the very last one. You have my ACK, but since John was > opposed maybe we should ask for his opinion too (so that we don't go > behind his back). > I still see no purpose for an alias to be saved since it's static, but I seem to be alone in that thinking. Just as much as I was alone in the thinking that qemuDomainGetManagedPRAlias is unnecessary and that a constant static string would work just the same. I'm also not convinced that a non managed system should be supported, but I recall Michal noting something during one of the reviews that it was useful for some future work. Shouldn't at the very least the formatting of the path and alias only occur for !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE)... Would mean passing @flags to virStoragePRDefFormat. At the very least it'll make it obvious about it's only being formatted for the active/status guest. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/13] qemu: command: Move check whether PR manager object props need to be built
On 05/14/2018 06:45 AM, Peter Krempa wrote: > Move it out of the formatter function and let the caller decide this. s/formatter/format/ I cannot recall our current consistency... Some times we seem to have the lowest routine make the singular check and other times we have the caller make the check. > > Signed-off-by: Peter Krempa> --- > src/qemu/qemu_command.c | 9 +++-- > src/qemu/qemu_hotplug.c | 3 +++ > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index 7df9979cb2..c38dde5a60 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -9687,9 +9687,6 @@ qemuBuildPRManagerInfoProps(const virDomainDiskDef > *disk, > *propsret = NULL; > *aliasret = NULL; > > -if (!disk->src->pr) > -return 0; > - > if (virStoragePRDefIsManaged(disk->src->pr)) { > if (VIR_STRDUP(alias, qemuDomainGetManagedPRAlias()) < 0) > goto cleanup; > @@ -9725,6 +9722,9 @@ qemuBuildMasterPRCommandLine(virCommandPtr cmd, > for (i = 0; i < def->ndisks; i++) { > const virDomainDiskDef *disk = def->disks[i]; > > +if (!disk->src->pr) > +continue; > + > if (virStoragePRDefIsManaged(disk->src->pr)) { > if (managedAdded) > continue; > @@ -9735,9 +9735,6 @@ qemuBuildMasterPRCommandLine(virCommandPtr cmd, > if (qemuBuildPRManagerInfoProps(disk, , ) < 0) > goto cleanup; > > -if (!props) > -continue; > - This one seems unrelated... although at this point I'm not sure how it could happen... I'd say keep it as is unless Michal had some other reason or maybe remembers what it was there for originally. John > if (!(tmp = > virQEMUBuildObjectCommandlineFromJSON("pr-manager-helper", >alias, >props))) > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c > index 3a26876c10..6557711ec1 100644 > --- a/src/qemu/qemu_hotplug.c > +++ b/src/qemu/qemu_hotplug.c > @@ -395,6 +395,9 @@ qemuMaybeBuildPRManagerInfoProps(virDomainObjPtr vm, > *propsret = NULL; > *aliasret = NULL; > > +if (!disk->src->pr) > +return 0; > + > if (virStoragePRDefIsManaged(disk->src->pr) && > priv->prDaemonRunning) { > /* @disk requires qemu-pr-helper but there's already one running. */ > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 09/13] qemu: process: Change semantics of functions starting PR daemon
On 05/14/2018 06:45 AM, Peter Krempa wrote: > Libvirt only manages one PR daemon. This means that we don't need to > pass the 'disk' object and also rename the functions dealing with this > so that it's obvious we only deal with the managed PR daemon. > > Signed-off-by: Peter KrempaSomething strange happened here. > --- > src/qemu/qemu_hotplug.c | 6 +++--- > src/qemu/qemu_process.c | 37 - > src/qemu/qemu_process.h | 5 ++--- > 3 files changed, 21 insertions(+), 27 deletions(-) > [...] > int > -qemuProcessStartPRDaemon(virDomainObjPtr vm, > - const virDomainDiskDef *disk) > +qemuProcessStartManagedPRDaemon(virDomainObjPtr vm) > { > qemuDomainObjPrivatePtr priv = vm->privateData; > virQEMUDriverPtr driver = priv->driver; > @@ -2640,10 +2639,6 @@ qemuProcessStartPRDaemon(virDomainObjPtr vm, > const unsigned long long timeout = 50; /* ms */ > int ret = -1; > > -if (!virStoragePRDefIsManaged(disk->src->pr) || > -priv->prDaemonRunning) > -return 0; > - > cfg = virQEMUDriverGetConfig(driver); > > if (!virFileIsExecutable(cfg->prHelperName)) { > @@ -2734,7 +2729,7 @@ qemuProcessStartPRDaemon(virDomainObjPtr vm, > goto cleanup; > > priv->prDaemonRunning = true; > -ret = 1; > +ret = 0; Unrelated, but since callers only care about < 0, no big deal... I'm assuming this is a holder from some other path which used the function for a return value (e.g. like qemuDomainMaybeStartPRDaemon) at some point during development. No big deal - I don't expect this to be extracted, but was just paying attention ;-) John > cleanup: > if (ret < 0) { > virCommandAbort(cmd); > @@ -2754,22 +2749,22 @@ qemuProcessStartPRDaemon(virDomainObjPtr vm, > > [...] -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 08/13] qemu: Assign managed PR path when preparing storage source
On 05/14/2018 06:41 AM, Peter Krempa wrote: > Rather than always checking which path to use pre-assign it when > preparing storage source. > > This reduces the need to pass 'vm' around too much. For later use the > path can be retrieved from the status XML. > > Signed-off-by: Peter Krempa> --- > src/qemu/qemu_command.c | 18 +- > src/qemu/qemu_command.h | 3 +-- > src/qemu/qemu_domain.c | 35 ++- > src/qemu/qemu_domain.h | 3 +-- > src/qemu/qemu_hotplug.c | 2 +- > src/qemu/qemu_process.c | 2 +- > 6 files changed, 31 insertions(+), 32 deletions(-) > Strange this patch got posted twice once w/ your own CC and once without. > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index 2bdba7734a..7df9979cb2 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -9667,7 +9667,6 @@ qemuBuildPanicCommandLine(virCommandPtr cmd, > > /** > * qemuBuildPRManagerInfoProps: > - * @vm: domain object > * @disk: disk definition > * @propsret: Returns JSON object containing properties of the > pr-manager-helper object > * @aliasret: alias of the pr-manager-helper object > @@ -9678,12 +9677,10 @@ qemuBuildPanicCommandLine(virCommandPtr cmd, > * -1 on failure (with error message set). > */ > int > -qemuBuildPRManagerInfoProps(virDomainObjPtr vm, > -const virDomainDiskDef *disk, > +qemuBuildPRManagerInfoProps(const virDomainDiskDef *disk, > virJSONValuePtr *propsret, > char **aliasret) > { > -char *socketPath = NULL; > char *alias = NULL; > int ret = -1; > > @@ -9693,9 +9690,6 @@ qemuBuildPRManagerInfoProps(virDomainObjPtr vm, > if (!disk->src->pr) > return 0; > > -if (!(socketPath = qemuDomainGetPRSocketPath(vm, disk->src->pr))) > -return ret; > - > if (virStoragePRDefIsManaged(disk->src->pr)) { > if (VIR_STRDUP(alias, qemuDomainGetManagedPRAlias()) < 0) > goto cleanup; > @@ -9705,7 +9699,7 @@ qemuBuildPRManagerInfoProps(virDomainObjPtr vm, > } > > if (virJSONValueObjectCreate(propsret, > - "s:path", socketPath, > + "s:path", disk->src->pr->path, > NULL) < 0) > goto cleanup; > > @@ -9713,14 +9707,12 @@ qemuBuildPRManagerInfoProps(virDomainObjPtr vm, > ret = 0; > cleanup: > VIR_FREE(alias); > -VIR_FREE(socketPath); > return ret; > } > > > static int > -qemuBuildMasterPRCommandLine(virDomainObjPtr vm, > - virCommandPtr cmd, > +qemuBuildMasterPRCommandLine(virCommandPtr cmd, > const virDomainDef *def) > { > size_t i; > @@ -9740,7 +9732,7 @@ qemuBuildMasterPRCommandLine(virDomainObjPtr vm, > managedAdded = true; > } > > -if (qemuBuildPRManagerInfoProps(vm, disk, , ) < 0) > +if (qemuBuildPRManagerInfoProps(disk, , ) < 0) > goto cleanup; > > if (!props) > @@ -9934,7 +9926,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, > if (qemuBuildMasterKeyCommandLine(cmd, priv) < 0) > goto error; > > -if (qemuBuildMasterPRCommandLine(vm, cmd, def) < 0) > +if (qemuBuildMasterPRCommandLine(cmd, def) < 0) Rather than @vm - what was really desired is/was @priv, which is already passed for qemuBuildMasterKeyCommandLine and could be for this too... Thus not requiring the hunks that change path in disk->src->pr->path. It is a static path beyond the priv->libDir part. > goto error; > > if (enableFips) > diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h > index da1fe679fe..621592cd79 100644 > --- a/src/qemu/qemu_command.h > +++ b/src/qemu/qemu_command.h > @@ -55,8 +55,7 @@ virCommandPtr qemuBuildCommandLine(virQEMUDriverPtr driver, > int **nicindexes); > > /* Generate the object properties for pr-manager */ > -int qemuBuildPRManagerInfoProps(virDomainObjPtr vm, > -const virDomainDiskDef *disk, > +int qemuBuildPRManagerInfoProps(const virDomainDiskDef *disk, > virJSONValuePtr *propsret, > char **alias); > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index eaa796260c..92217e66fe 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -11930,6 +11930,22 @@ qemuDomainPrepareDiskCachemode(virDomainDiskDefPtr > disk) > } > > > +static int > +qemuDomainPrepareStorageSourcePR(virStorageSourcePtr src, > + qemuDomainObjPrivatePtr priv) > +{ > +if (!src->pr) > +return 0; > + > +if (virStoragePRDefIsManaged(src->pr)) { > +if (!(src->pr->path = qemuDomainGetManagedPRSocketPath(priv))) > +return -1; > +}
Re: [libvirt] [PATCH 05/13] util: storage: Drop pointless 'enabled' form PR definition
On 05/14/2018 06:41 AM, Peter Krempa wrote: > Everything can be disabled by not using the parent element. There's no > need to store this explicitly. Additionally it does not add any value > since any configuration is dropped if enabled='no' is configured. > > Drop the attribute and adjust the code accordingly.t > > Signed-off-by: Peter Krempa> --- > docs/formatdomain.html.in | 21 ++-- > docs/schemas/storagecommon.rng | 3 - > src/util/virstoragefile.c | 117 > + > src/util/virstoragefile.h | 1 - > .../disk-virtio-scsi-reservations.xml | 4 +- > 5 files changed, 59 insertions(+), 87 deletions(-) > As I've worked may way forward - I reread the docs and needed to return here for commenting... > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in > index 80172c18d0..d69a669259 100644 > --- a/docs/formatdomain.html.in > +++ b/docs/formatdomain.html.in > @@ -2583,7 +2583,7 @@ >disk type='block' device='lun' > driver name='qemu' type='raw'/ > source dev='/dev/sda' > - reservations enabled='yes' managed='no' > + reservations managed='no' > source type='unix' path='/path/to/qemu-pr-helper' > mode='client'/ >/reservations > target dev='sda' bus='scsi'/ > @@ -2952,17 +2952,16 @@ >Since libvirt 4.4.0, the > reservations can be a sub-element of the > source element for storage sources (QEMU driver > only). > -If present (and enabled) it enables persistent reservations for > SCSI > +If present it enables persistent reservations for SCSI > based disks. The element has one mandatory attribute > -enabled with accepted values yes and > -no. If the feature is enabled, then there's another > -mandatory attribute managed (accepted values are the > -same as for enabled) that enables or disables > libvirt > -spawning a helper process. When the PR is unmanaged, then > hypervisor > -acts as a client and path to server socket must be provided in > child > -element source, which currently accepts only the > -following attributes: type with one value > -unix, path with path the socket, and > +managed with accepted values yes and > +no. If managed is enabled libvirt > prepares > +and manages any resources needed for the feature. When the PR is s/for the feature././ s/PR is/persistent reservations are/ > +unmanaged, then hypervisor acts as a client and path to server s/then/then the/ s/and path to server/and the path to the server/ > +socket must be provided in child element source, s/in child/in the child/ > +which currently accepts only the following attributes: > +type with one value unix, > +path with path the socket, and s/with path the socket/path to the socket/ > finally mode which accepts one value > client and specifies the role of hypervisor. s/and specifies/specifying > It's recommended to allow libvirt manage the persistent John [...] -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 05/13] util: storage: Drop pointless 'enabled' form PR definition
On 05/14/2018 06:41 AM, Peter Krempa wrote: > Everything can be disabled by not using the parent element. There's no > need to store this explicitly. Additionally it does not add any value > since any configuration is dropped if enabled='no' is configured. > > Drop the attribute and adjust the code accordingly.t > > Signed-off-by: Peter Krempa> --- > docs/formatdomain.html.in | 21 ++-- > docs/schemas/storagecommon.rng | 3 - > src/util/virstoragefile.c | 117 > + > src/util/virstoragefile.h | 1 - > .../disk-virtio-scsi-reservations.xml | 4 +- > 5 files changed, 59 insertions(+), 87 deletions(-) > Hmm... I recall stating the same thing during v1 and v2 review, but got a deafening the design of XML was agreed upon already... John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] bhyve: allow locking memory
On 14 May 2018, at 8:51, Roman Bogorodskiy wrote: > Peter Krempa wrote: > >> On Sun, May 13, 2018 at 14:01:25 +0400, Roman Bogorodskiy wrote: >>> Fabian Freyer wrote: >> >> [...] >> 13 files changed, 134 insertions(+) create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-wired.args create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-wired.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-wired.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-wired.ldargs create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-wired.xml create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-wired.xml >>> >>> Thanks for the patches, I've pushed the series with some syntax-check >>> fixes (trailing whitespaces, tabs instead of whitespaces) and added >>> 'Signed-off-by' line which is mandatory now. >> >> Note that 'Signed-off-by' should _NOT_ be added without explicit consent >> of the author of the patches. Otherwise it defeats the purpose of >> certification that the author agrees and complies with the Developer >> certificate of origin. Adding it without explicit consent makes the >> Signed-off-by line just a useless piece of jewelery added to appease the >> push hook. > > Oh, sorry about that. I assumed as Fabian is a long time contributor, > he's familiar with the general project policies. > > Fabian, do you have any issue with that? All good with me. In case you need it from me, for the whole series: Signed-off-by: Fabian Freyersignature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/8] qemu: vfio-ccw device address generation
On 05/14/2018 03:43 AM, Boris Fiuczynski wrote: > On 05/10/2018 10:52 PM, John Ferlan wrote: >> >> >> On 05/07/2018 10:41 AM, Boris Fiuczynski wrote: >>> From: Shalini Chellathurai Saroja>>> >>> Introduces the vfio-ccw model for mediated devices and prime vfio-ccw >>> devices such that CCW address will be generated. >>> >>> Signed-off-by: Shalini Chellathurai Saroja >>> Reviewed-by: Bjoern Walk >>> Reviewed-by: Boris Fiuczynski >>> Reviewed-by: Marc Hartmayer >>> Reviewed-by: Stefan Zimmermann >>> --- >>> docs/schemas/domaincommon.rng | 5 - >>> src/qemu/qemu_domain_address.c | 20 >>> src/util/virmdev.c | 3 ++- >>> src/util/virmdev.h | 1 + >>> 4 files changed, 27 insertions(+), 2 deletions(-) >>> >> >> Looking at all places using VIR_MDEV_MODEL_TYPE_VFIO_PCI - should this >> patch make a change to virDomainHostdevDefPostParse to do something >> similar - that is: >> >> if (model == VIR_MDEV_MODEL_TYPE_VFIO_CCW && >> dev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { >> ... error message >> } >> >> ? >> >> Let me know... I think it should and can add it before pushing... > You are correct. Good catch! > How about fixing it like this? > if ((model == VIR_MDEV_MODEL_TYPE_VFIO_PCI && > dev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) || > (model == VIR_MDEV_MODEL_TYPE_VFIO_CCW && > dev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) { > virReportError(VIR_ERR_XML_ERROR, > _("Unsupported address type '%s' with mediated " > "device model '%s'"), > > virDomainDeviceAddressTypeToString(dev->info->type), > virMediatedDeviceModelTypeToString(model)); > return -1; > } > OK - added that... > > > Besides that I just saw that the indention of the second parameter of > method qemuDomainPrimeVfioDeviceAddresses is off by three blanks. > ah true - adjusted that. I've merged with the latest top of tree and have pushed... So much flux in the capabilities lately... John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] Introduce virCryptoHashBuf
On 05/11/2018 11:50 AM, Ján Tomko wrote: A function that keeps the hash in binary form instead of converting it to human-readable hexadecimal form. Signed-off-by: Ján Tomko--- src/util/vircrypto.c | 31 +-- src/util/vircrypto.h | 7 +++ 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/util/vircrypto.c b/src/util/vircrypto.c index 48b04fc8ce..1a2dcc28b7 100644 --- a/src/util/vircrypto.c +++ b/src/util/vircrypto.c @@ -54,28 +54,39 @@ struct virHashInfo { verify(ARRAY_CARDINALITY(hashinfo) == VIR_CRYPTO_HASH_LAST); int -virCryptoHashString(virCryptoHash hash, -const char *input, -char **output) +virCryptoHashBuf(virCryptoHash hash, + const char *input, + unsigned char *output) { -unsigned char buf[VIR_CRYPTO_LARGEST_DIGEST_SIZE]; -size_t hashstrlen; -size_t i; - if (hash >= VIR_CRYPTO_HASH_LAST) { virReportError(VIR_ERR_INVALID_ARG, _("Unknown crypto hash %d"), hash); return -1; } -hashstrlen = (hashinfo[hash].hashlen * 2) + 1; - -if (!(hashinfo[hash].func(input, strlen(input), buf))) { +if (!(hashinfo[hash].func(input, strlen(input), output))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unable to compute hash of data")); return -1; } +return 0; +} + +int +virCryptoHashString(virCryptoHash hash, +const char *input, +char **output) +{ +unsigned char buf[VIR_CRYPTO_LARGEST_DIGEST_SIZE]; +size_t hashstrlen; +size_t i; + +if (virCryptoHashBuf(hash, input, buf) < 0) +return -1; + +hashstrlen = (hashinfo[hash].hashlen * 2) + 1; + How about virCryptoHashBuf returning the number of raw bytes it produced so that not all callers need to look it up? Stefan if (VIR_ALLOC_N(*output, hashstrlen) < 0) return -1; diff --git a/src/util/vircrypto.h b/src/util/vircrypto.h index 81743d2f74..64984006be 100644 --- a/src/util/vircrypto.h +++ b/src/util/vircrypto.h @@ -41,6 +41,13 @@ typedef enum { VIR_CRYPTO_CIPHER_LAST } virCryptoCipher; +int +virCryptoHashBuf(virCryptoHash hash, + const char *input, + unsigned char *output) +ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) +ATTRIBUTE_RETURN_CHECK; + int virCryptoHashString(virCryptoHash hash, const char *input, -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 05/13] util: storage: Drop pointless 'enabled' form PR definition
On 05/14/2018 12:41 PM, Peter Krempa wrote: > Everything can be disabled by not using the parent element. There's no > need to store this explicitly. Additionally it does not add any value > since any configuration is dropped if enabled='no' is configured. > > Drop the attribute and adjust the code accordingly.t s/t$// > > Signed-off-by: Peter Krempa> --- > docs/formatdomain.html.in | 21 ++-- > docs/schemas/storagecommon.rng | 3 - > src/util/virstoragefile.c | 117 > + > src/util/virstoragefile.h | 1 - > .../disk-virtio-scsi-reservations.xml | 4 +- > 5 files changed, 59 insertions(+), 87 deletions(-) Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/13] Fix XML for persistent reservations and refactor
On 05/14/2018 12:41 PM, Peter Krempa wrote: > The XML design for the PR stuff is slightly weird so fix it and refactor > the code so that it will be much easier to use it with the blockdev > infrastructure. > > Peter Krempa (13): > qemu: hotplug: Fix spacing around addition operator > qemu: alias: Allow passing alias of parent when generating PR manager > alias > qemu: command: Fix comment for qemuBuildPRManagerInfoProps > qemu: Move validation of PR manager support > util: storage: Drop pointless 'enabled' form PR definition > util: storage: Drop virStoragePRDefIsEnabled > util: storage: Allow passing also for managed PR case > qemu: Assign managed PR path when preparing storage source > qemu: process: Change semantics of functions starting PR daemon > qemu: command: Move check whether PR manager object props need to be > built > conf: domain: Add helper to check whether a domain def requires use of > PR > util: storage: Store PR manager alias in the definition > qemu: hotplug: Replace qemuDomainDiskNeedRemovePR > > docs/formatdomain.html.in | 21 ++-- > docs/schemas/storagecommon.rng | 3 - > src/conf/domain_conf.c | 21 > src/conf/domain_conf.h | 3 + > src/libvirt_private.syms | 2 +- > src/qemu/qemu_alias.c | 4 +- > src/qemu/qemu_alias.h | 2 +- > src/qemu/qemu_command.c| 61 +++--- > src/qemu/qemu_command.h| 6 +- > src/qemu/qemu_domain.c | 66 --- > src/qemu/qemu_domain.h | 3 +- > src/qemu/qemu_hotplug.c| 83 +++--- > src/qemu/qemu_process.c| 40 ++- > src/qemu/qemu_process.h| 5 +- > src/util/virstoragefile.c | 124 > - > src/util/virstoragefile.h | 5 +- > tests/qemustatusxml2xmldata/modern-in.xml | 4 + > .../disk-virtio-scsi-reservations.xml | 4 +- > tests/qemuxml2xmltest.c| 2 +- > 19 files changed, 191 insertions(+), 268 deletions(-) > You have my ACK on the series, but I guess we'll need John's input on 12/13 because he was opposed to saving anything in status XML. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 12/13] util: storage: Store PR manager alias in the definition
On 05/14/2018 12:45 PM, Peter Krempa wrote: > Rather than always re-generating the alias store it in the definition > and in the status XML. > > Signed-off-by: Peter Krempa> --- > src/qemu/qemu_command.c | 23 +++-- > src/qemu/qemu_command.h | 3 +-- > src/qemu/qemu_domain.c| 16 +-- > src/qemu/qemu_hotplug.c | 34 > ++- > src/util/virstoragefile.c | 1 + > src/util/virstoragefile.h | 3 +++ > tests/qemustatusxml2xmldata/modern-in.xml | 4 > 7 files changed, 37 insertions(+), 47 deletions(-) Yes, this makes sense to me. I've kept alias in status XML for all the versions until the very last one. You have my ACK, but since John was opposed maybe we should ask for his opinion too (so that we don't go behind his back). Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] ANNOUNCE: Plans for the next release
Hi, I would like to make the next libvirt-dbus release with stable APIs, there is a lot of them already implemented and other projects can start adapting to use libvirt-dbus. The release version would be 1.0.0 and currently libvirt-dbus will cover APIs up to libvirt-3.0.0 as a start point because that libvirt version is available in all downstream distributions currently supported by libvirt. So I would like to ask everyone to look at the APIs to check whether it make sense and whether something is missing or could be excluded. The idea of libvirt-dbus is not to export every API if there is a better API to do the job, so for example: - all APIs that have both versions with and without flags we will export only the one with flags. - some of the Stats APIs can be omitted since the same values are also exported in the GetStats (virDomainListGetStats) I would like to make the release this week so any help or suggestion is welcome. The API definition is in the data/org.libvirt.*.xml files. Thanks and regards Pavel signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/12] Parse domain XML to generate virDomainLoaderDef & virDomainNvramDef.
On Mon, May 14, 2018 at 16:20:53 +0200, Marc Hartmayer wrote: > On Mon, May 14, 2018 at 01:15 PM +0200, Prerna Saxena> wrote: > > This patch is used to interpret domain XML and store the Loader & > > Nvram's backing definitions as virStorageSource. > > It also identifies if input XML used old or new-style declaration. > > (This will later be used by the formatter). > > > > Signed-off-by: Prerna Saxena > > --- > > src/conf/domain_conf.c | 131 > > ++--- > > 1 file changed, 124 insertions(+), 7 deletions(-) > > > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > > index f678e26..be43695 100644 > > --- a/src/conf/domain_conf.c > > +++ b/src/conf/domain_conf.c > > @@ -2884,8 +2884,8 @@ virDomainLoaderDefFree(virDomainLoaderDefPtr loader) > > if (!loader) > > return; > > > > -VIR_FREE(loader->path); > > -VIR_FREE(loader->nvram); > > +virStorageSourceFree(loader->src); > > +virStorageSourceFree(loader->nvram); > > VIR_FREE(loader->templt); > > VIR_FREE(loader); > > } > > @@ -17986,17 +17986,62 @@ > > virDomainDefMaybeAddHostdevSCSIcontroller(virDomainDefPtr def) > > > > static int > > virDomainLoaderDefParseXML(xmlNodePtr node, > > + xmlXPathContextPtr ctxt, > > virDomainLoaderDefPtr loader) > > { > > int ret = -1; > > char *readonly_str = NULL; > > char *secure_str = NULL; > > char *type_str = NULL; > > +char *tmp = NULL; > > +xmlNodePtr cur; > > +xmlXPathContextPtr cur_ctxt = ctxt; > > + > > +if (VIR_ALLOC(loader->src)) { > ^^ > Usually it is checked for < 0. > > > +goto cleanup; > > +} > > +loader->src->type = VIR_STORAGE_TYPE_LAST; > > +loader->oldStyleLoader = false; > > > > readonly_str = virXMLPropString(node, "readonly"); > > secure_str = virXMLPropString(node, "secure"); > > type_str = virXMLPropString(node, "type"); > > -loader->path = (char *) xmlNodeGetContent(node); > > + > > +if ((tmp = virXMLPropString(node, "backing")) && > > +(loader->src->type = virStorageTypeFromString(tmp)) <= 0) { > > If virStorageTypeFromString fails there is a memory leak of @tmp. > > > +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > > + _("unknown loader type '%s'"), tmp); > > +goto cleanup; > > +} > > +VIR_FREE(tmp); > > + > > +for (cur = node->children; cur != NULL; cur = cur->next) { > > +if (cur->type != XML_ELEMENT_NODE) { > > +continue; > > +} > > + > > +if (virXMLNodeNameEqual(cur, "source")) { > > +/* new-style declaration found */ > > +if (virDomainStorageSourceParse(cur, cur_ctxt, loader->src, 0) > > < 0) { > > +virReportError(VIR_ERR_XML_DETAIL, > > + _("Error parsing Loader source element")); > > +goto cleanup; > > +} > > +break; > > +} > > +} > > + > > +/* Old-style absolute path found ? */ > > +if (loader->src->type == VIR_STORAGE_TYPE_LAST) { > > +if (!(loader->src->path = (char *) xmlNodeGetContent(node))) { > > +virReportError(VIR_ERR_XML_ERROR, "%s", > > + _("missing loader source")); > > +goto cleanup; > > +} else { > > +loader->src->type = VIR_STORAGE_TYPE_FILE; > > +loader->oldStyleLoader = true; > > +} > > +} > > > > if (readonly_str && > > (loader->readonly = virTristateBoolTypeFromString(readonly_str)) > > <= 0) { > > @@ -18023,13 +18068,81 @@ virDomainLoaderDefParseXML(xmlNodePtr node, > > } > > > > ret = 0; > > - cleanup: > > +goto exit; > > +cleanup: > > I would replace: s/cleanup/error and s/exit/cleanup. > > > +if (loader->src) > > + VIR_FREE(loader->src); > > Shouldn’t it be a virStorageSourceFree() here? Not sure if it’s needed > but it’s safer. Additionally the preferred way is to use a second virStorageSourcePtr to hold the data while it's being parsed and then use VIR_STEAL_PTR to put it into the structure. The cleanup section then can call virStorageSourceFree on the temp ptr unconditionally and it also avoids this messy labels. > > > +exit: > > VIR_FREE(readonly_str); > > VIR_FREE(secure_str); > > VIR_FREE(type_str); > > + > > return ret; > > } > > > > +static int > > +virDomainLoaderNvramDefParseXML(xmlNodePtr node, > > + xmlXPathContextPtr ctxt, > > + virDomainLoaderDefPtr loader) > > +{ > > +int ret = -1; > > +char *tmp = NULL; > > +xmlNodePtr cur; > > + > > +if (VIR_ALLOC(loader->nvram)) { >^^ >Usually it is
Re: [libvirt] [PATCH] travis: Uninstall packages before upgrade
On Mon, 2018-05-14 at 16:12 +0200, Michal Privoznik wrote: > This OSX support is becoming more and more hairy. It's fairly often > broken and we do nothing but compile test it (we don't even run make > check there). So we can't be really sure the compiled virsh/client > library still works there. I think it's time to have a discussion about > dropping OSX support. Do we know if we even have any consumers running > libvirt on OSX? According to https://brew.sh/analytics/install/ there's some 20K people with libvirt installed on their macOS machine; I think it's fair to assume at least a fraction of them are actually using it. Compiling on non-Linux platforms allows us to keep libvirt farily portable, so having FreeBSD, MinGW and macOS builds running as part of our CI setup is IMHO extremely valuable. I consider macOS very much a "best effort" kind of deal, since it's basically impossible to solve all but the most trivial issues hitting it without first gaining access to proprietary platforms, which is something that I'm personally not interested in doing. That said, as long as we can keep it building by tweaking a couple of lines every few months, I don't see a compelling argument for dropping macOS support. -- Andrea Bolognani / Red Hat / Virtualization -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/12] Parse domain XML to generate virDomainLoaderDef & virDomainNvramDef.
On Mon, May 14, 2018 at 01:15 PM +0200, Prerna Saxenawrote: > This patch is used to interpret domain XML and store the Loader & > Nvram's backing definitions as virStorageSource. > It also identifies if input XML used old or new-style declaration. > (This will later be used by the formatter). > > Signed-off-by: Prerna Saxena > --- > src/conf/domain_conf.c | 131 > ++--- > 1 file changed, 124 insertions(+), 7 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index f678e26..be43695 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -2884,8 +2884,8 @@ virDomainLoaderDefFree(virDomainLoaderDefPtr loader) > if (!loader) > return; > > -VIR_FREE(loader->path); > -VIR_FREE(loader->nvram); > +virStorageSourceFree(loader->src); > +virStorageSourceFree(loader->nvram); > VIR_FREE(loader->templt); > VIR_FREE(loader); > } > @@ -17986,17 +17986,62 @@ > virDomainDefMaybeAddHostdevSCSIcontroller(virDomainDefPtr def) > > static int > virDomainLoaderDefParseXML(xmlNodePtr node, > + xmlXPathContextPtr ctxt, > virDomainLoaderDefPtr loader) > { > int ret = -1; > char *readonly_str = NULL; > char *secure_str = NULL; > char *type_str = NULL; > +char *tmp = NULL; > +xmlNodePtr cur; > +xmlXPathContextPtr cur_ctxt = ctxt; > + > +if (VIR_ALLOC(loader->src)) { ^^ Usually it is checked for < 0. > +goto cleanup; > +} > +loader->src->type = VIR_STORAGE_TYPE_LAST; > +loader->oldStyleLoader = false; > > readonly_str = virXMLPropString(node, "readonly"); > secure_str = virXMLPropString(node, "secure"); > type_str = virXMLPropString(node, "type"); > -loader->path = (char *) xmlNodeGetContent(node); > + > +if ((tmp = virXMLPropString(node, "backing")) && > +(loader->src->type = virStorageTypeFromString(tmp)) <= 0) { If virStorageTypeFromString fails there is a memory leak of @tmp. > +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("unknown loader type '%s'"), tmp); > +goto cleanup; > +} > +VIR_FREE(tmp); > + > +for (cur = node->children; cur != NULL; cur = cur->next) { > +if (cur->type != XML_ELEMENT_NODE) { > +continue; > +} > + > +if (virXMLNodeNameEqual(cur, "source")) { > +/* new-style declaration found */ > +if (virDomainStorageSourceParse(cur, cur_ctxt, loader->src, 0) < > 0) { > +virReportError(VIR_ERR_XML_DETAIL, > + _("Error parsing Loader source element")); > +goto cleanup; > +} > +break; > +} > +} > + > +/* Old-style absolute path found ? */ > +if (loader->src->type == VIR_STORAGE_TYPE_LAST) { > +if (!(loader->src->path = (char *) xmlNodeGetContent(node))) { > +virReportError(VIR_ERR_XML_ERROR, "%s", > + _("missing loader source")); > +goto cleanup; > +} else { > +loader->src->type = VIR_STORAGE_TYPE_FILE; > +loader->oldStyleLoader = true; > +} > +} > > if (readonly_str && > (loader->readonly = virTristateBoolTypeFromString(readonly_str)) <= > 0) { > @@ -18023,13 +18068,81 @@ virDomainLoaderDefParseXML(xmlNodePtr node, > } > > ret = 0; > - cleanup: > +goto exit; > +cleanup: I would replace: s/cleanup/error and s/exit/cleanup. > +if (loader->src) > + VIR_FREE(loader->src); Shouldn’t it be a virStorageSourceFree() here? Not sure if it’s needed but it’s safer. > +exit: > VIR_FREE(readonly_str); > VIR_FREE(secure_str); > VIR_FREE(type_str); > + > return ret; > } > > +static int > +virDomainLoaderNvramDefParseXML(xmlNodePtr node, > + xmlXPathContextPtr ctxt, > + virDomainLoaderDefPtr loader) > +{ > +int ret = -1; > +char *tmp = NULL; > +xmlNodePtr cur; > + > +if (VIR_ALLOC(loader->nvram)) { ^^ Usually it is checked for < 0. > +goto cleanup; > +} Unneeded braces. > + > +loader->nvram->type = VIR_STORAGE_TYPE_LAST; > +loader->nvram->oldStyleNvram = false; > + > +if ((tmp = virXMLPropString(node, "backing")) && > +(loader->nvram->type = virStorageTypeFromString(tmp)) <= 0) { If virStorageTypeFromString fails there is a memory leak of @tmp. > +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, > + _("unknown nvram type '%s'"), tmp); > +goto cleanup; > +} > +VIR_FREE(tmp); > + > +for (cur = node->children; cur != NULL; cur = cur->next) { > +if
Re: [libvirt] [PATCH] travis: Uninstall packages before upgrade
On 05/14/2018 12:07 PM, Andrea Bolognani wrote: > numpy (needed by cgal) started having the same issue with > linking as python, which makes upgrade and thus the entire > build fail on macOS. > > Instead of playing more tricks with linking/unlinking, just > uninstall the problematic packages (and those dragging them > in) before doing anything else. > > Signed-off-by: Andrea Bolognani> --- > Technically a build breaker, but since I'm changing the > approach I'd rather wait for an explicit ACK before pushing. > > .travis.yml | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/.travis.yml b/.travis.yml > index d3f72d46f3..140072b801 100644 > --- a/.travis.yml > +++ b/.travis.yml > @@ -21,10 +21,10 @@ matrix: > - compiler: clang >os: osx >before_install: > +- brew uninstall python mercurial postgis sfcgal cgal gdal > - brew update > -- brew unlink python > - brew upgrade > -- brew install rpcgen yajl > +- brew install python rpcgen yajl >script: > # We can't run make distcheck/syntax-check because they > # fail on macOS, but doing 'install' and 'dist' gives us > This OSX support is becoming more and more hairy. It's fairly often broken and we do nothing but compile test it (we don't even run make check there). So we can't be really sure the compiled virsh/client library still works there. I think it's time to have a discussion about dropping OSX support. Do we know if we even have any consumers running libvirt on OSX? Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tests: Fix qemucapsprobemock linking
On 05/10/2018 11:13 PM, Martin Kletzander wrote: > Add libvirt.la into qemucapsprobemock_la_LIBADD because that mock uses bunch > of > libvirt internal functions. Without this patch the following error occurs: > > $ ./qemucapsprobe /usr/bin/qemu-system-x86_64 > ... > warning : virQEMUCapsLogProbeFailure:4204 : Failed to probe capabilities for > /usr/bin/qemu-system-x86_64: internal error: Failed to probe QEMU binary > with > QMP: /usr/bin/qemu-system-x86_64: symbol lookup error: > .../qemucapsprobemock.so: undefined symbol: virJSONValueObjectHasKey > > Signed-off-by: Martin Kletzander> --- > > TBH I didn't go into details why this is needed. It still seems fishy to me. > Not only that I get that error above as a libvirt error, but if i add just > libvirt_util.la in there, I get: > > .../qemucapsprobe: symbol lookup error: .../qemucapsprobemock.so: > undefined symbol: libvirt_event_poll_update_handle_semaphore This is because libvirt_utils is built WITH_DTRACE_PROBES enabled. Therefore it requires libvirt_probes.lo. So adding these two should be enough. However, I'm unable to reproduce this. But what I am able to reproduce is: libvirt.git/tests $ ../run valgrind --trace-children=yes ./qemuxml2argvtest Missing symbol 'virFileCanonicalizePath' Aborted (core dumped) This points to merely the same problem. In our mocks we are/want to use our internal APIs. Therefore instead of relying on program that links with the mock at runtime (VIR_TEST_MAIN_PRELOAD macro) being already linked with libvirt.so I think we should link our mocks with it: diff --git i/tests/Makefile.am w/tests/Makefile.am index 621480dd0c..ac92190845 100644 --- i/tests/Makefile.am +++ w/tests/Makefile.am @@ -81,7 +81,8 @@ LDADDS = \ ../src/libvirt.la MOCKLIBS_LIBS = \ - $(GNULIB_LIBS) + $(GNULIB_LIBS) \ + ../src/libvirt.la EXTRA_DIST = \ .valgrind.supp \ Michal P.S. This opened a pandora box for me when I tried running qemucapsprobe over the latest qemu compiled from git only to find that qemu has a regression when it does not accept -no-user-config on the command line anymore. Patch proposed upstream (link not yet available because their list is very slow). -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] log: actually do substring matches with fnmatch
Historically we matched log filters with strstr(), and when switching to fnmatch in cbb0fd3cfdc287f6f4653ef1f04a7cfb2ea51b27, it was stated that we would continue to match substrings, with "foo" being equivalent to "*foo*". Unfortuntely I forget to provide the code to actually make that happen. This fixes it to prepend and append "*" if not already present in the user's pattern. Signed-off-by: Daniel P. Berrangé--- src/util/virlog.c | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/util/virlog.c b/src/util/virlog.c index be9fc0cf78..d548010b10 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -1409,6 +1409,8 @@ virLogFilterNew(const char *match, { virLogFilterPtr ret = NULL; char *mdup = NULL; +size_t mlen = strlen(match); +size_t off = 0; virCheckFlags(VIR_LOG_STACK_TRACE, NULL); @@ -1418,9 +1420,19 @@ virLogFilterNew(const char *match, return NULL; } -if (VIR_STRDUP_QUIET(mdup, match) < 0) +if (VIR_ALLOC_N_QUIET(mdup, mlen + 3) < 0) return NULL; +if (match[0] != '*') { +mdup[off++] = '*'; +} +memcpy(mdup + off, match, mlen + 1); +off += mlen; +if (match[mlen - 1] != '*') { +mdup[off++] = '*'; +mdup[off++] = '\0'; +} + if (VIR_ALLOC_QUIET(ret) < 0) { VIR_FREE(mdup); return NULL; -- 2.17.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/5] qemu: Add gl property to graphics of type sdl in domain config
On 05/14/2018 09:18 AM, Maciej Wolny wrote: > On 14/05/18 13:40, Martin Kletzander wrote: >> On Mon, May 14, 2018 at 08:27:35AM -0400, John Ferlan wrote: >>> >>> >>> On 05/14/2018 07:24 AM, Martin Kletzander wrote: On Fri, May 11, 2018 at 03:09:20PM +0100, Maciej Wolny wrote: > On 11/05/18 09:42, Martin Kletzander wrote: >> On Thu, May 10, 2018 at 11:53:57AM +0100, Maciej Wolny wrote: >>> Support OpenGL accelerated rendering when using SDL graphics in the >>> domain config. Add associated test and documentation. >>> >>> Signed-off-by: Maciej Wolny>>> --- >>> docs/formatdomain.html.in | 6 +++ >>> docs/schemas/domaincommon.rng | 8 >>> src/conf/domain_conf.c | 44 >>> - >>> src/conf/domain_conf.h | 1 + >> >> docs, conf and schemas fit together nicely, they should be in one >> patch, but. >> >>> tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 >>> ++ >>> .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 >>> ++ >>> tests/qemuxml2xmltest.c | 1 + >> >> this has nothing to do with qemu (yet), also see Subject (I wouldn't say >> 'qemu:' there, but rather something like 'docs, conf, schema:') >> >> For the XML tests above you can use genericxml2xmltest instead of the >> QEMU-specific one. > > The option only makes sense in QEMU afaik, hence the naming. > Yes, for now. If someone who's building the code without QEMU driver changes the behaviour, the tests will pass if you keep it in qemuxml2xml, however genericxml2xml will catch that. qemuxml2xml should be testing specifics where you behave based on some more information than just generic XML. I hope that's clear. Have a nice day. >>> >>> However, until qemuxml2argvtest can also pull out of genericxml2xmldata, >>> then you'd have separate xml input and output files - is that what's >>> desired? >>> >>> Taking a quick look just now - see the graphics-vnc-socket - do we want >>> to duplicate having two input/output XML files which invariably will >>> diverge? Ironically the generic one has a domain type == qemu, an >>> emulator using qemu, and the socket path using QEMU - so while it's >>> generic in one sense, it's not in others. Even more ironic is the qemu >>> specific file uses "". >>> >>> Could/should generification of the xml2xml tests be considered a "bite >>> sized task"? >>> >> >> Oh, definitely. It's only some time ago that the tests started to be usable >> IIRC, so hopefully we'll migrate some XMLs here and there. But maybe others >> could chime in as well so that I don't speak for others. I remember Pavel >> having some ideas for cleaner separation of those. > > So, do you guys want to leave that for a separate patch set or do you want me > to > post a v3 with the changes Martin has requested? > My opinion is leave it as is - hard to require you to do something we're not requiring other patches at this point. I'm not a fan of duplication - that is the task would be to essentially copy everything into the generic xml2xml test at this point. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/5] qemu: Add gl property to graphics of type sdl in domain config
On 14/05/18 13:40, Martin Kletzander wrote: > On Mon, May 14, 2018 at 08:27:35AM -0400, John Ferlan wrote: >> >> >> On 05/14/2018 07:24 AM, Martin Kletzander wrote: >>> On Fri, May 11, 2018 at 03:09:20PM +0100, Maciej Wolny wrote: On 11/05/18 09:42, Martin Kletzander wrote: > On Thu, May 10, 2018 at 11:53:57AM +0100, Maciej Wolny wrote: >> Support OpenGL accelerated rendering when using SDL graphics in the >> domain config. Add associated test and documentation. >> >> Signed-off-by: Maciej Wolny>> --- >> docs/formatdomain.html.in | 6 +++ >> docs/schemas/domaincommon.rng | 8 >> src/conf/domain_conf.c | 44 >> - >> src/conf/domain_conf.h | 1 + > > docs, conf and schemas fit together nicely, they should be in one > patch, but. > >> tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 >> ++ >> .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 >> ++ >> tests/qemuxml2xmltest.c | 1 + > > this has nothing to do with qemu (yet), also see Subject (I wouldn't say > 'qemu:' there, but rather something like 'docs, conf, schema:') > > For the XML tests above you can use genericxml2xmltest instead of the > QEMU-specific one. The option only makes sense in QEMU afaik, hence the naming. >>> >>> Yes, for now. If someone who's building the code without QEMU driver >>> changes >>> the behaviour, the tests will pass if you keep it in qemuxml2xml, however >>> genericxml2xml will catch that. qemuxml2xml should be testing specifics >>> where >>> you behave based on some more information than just generic XML. >>> >>> I hope that's clear. >>> >>> Have a nice day. >>> >> >> However, until qemuxml2argvtest can also pull out of genericxml2xmldata, >> then you'd have separate xml input and output files - is that what's >> desired? >> >> Taking a quick look just now - see the graphics-vnc-socket - do we want >> to duplicate having two input/output XML files which invariably will >> diverge? Ironically the generic one has a domain type == qemu, an >> emulator using qemu, and the socket path using QEMU - so while it's >> generic in one sense, it's not in others. Even more ironic is the qemu >> specific file uses "". >> >> Could/should generification of the xml2xml tests be considered a "bite >> sized task"? >> > > Oh, definitely. It's only some time ago that the tests started to be usable > IIRC, so hopefully we'll migrate some XMLs here and there. But maybe others > could chime in as well so that I don't speak for others. I remember Pavel > having some ideas for cleaner separation of those. So, do you guys want to leave that for a separate patch set or do you want me to post a v3 with the changes Martin has requested? -- milloni -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] Introduce virCryptoHashBuf
On Mon, May 14, 2018 at 11:17:51AM +0200, Michal Privoznik wrote: On 05/11/2018 05:50 PM, Ján Tomko wrote: A function that keeps the hash in binary form instead of converting it to human-readable hexadecimal form. Signed-off-by: Ján Tomko--- src/util/vircrypto.c | 31 +-- src/util/vircrypto.h | 7 +++ 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/util/vircrypto.c b/src/util/vircrypto.c index 48b04fc8ce..1a2dcc28b7 100644 --- a/src/util/vircrypto.c +++ b/src/util/vircrypto.c @@ -54,28 +54,39 @@ struct virHashInfo { verify(ARRAY_CARDINALITY(hashinfo) == VIR_CRYPTO_HASH_LAST); int -virCryptoHashString(virCryptoHash hash, -const char *input, -char **output) +virCryptoHashBuf(virCryptoHash hash, + const char *input, + unsigned char *output) { -unsigned char buf[VIR_CRYPTO_LARGEST_DIGEST_SIZE]; -size_t hashstrlen; -size_t i; - if (hash >= VIR_CRYPTO_HASH_LAST) { virReportError(VIR_ERR_INVALID_ARG, _("Unknown crypto hash %d"), hash); return -1; } This check feels useless. It's like if we were checking a value before passing it to vir*TypeToString(). But it's pre-existing, so you have my ACK. But a follow up patch removing it (=trivial) would be nice. On one hand, this check should be pointless if the rest of our code is correct. On the other hand, our functions in src/util usually do perform some validation of the parameters. Although it could be transformed to use virReportEnumRangeError. Also, don't forget to export the symbol in libvirt_private.syms ;-) I'm wondering how linker succeeds in 3/5 where you use the function without it being exported. Maybe my compiler decided to inline this function? Thanks, fixed. Jano Michal signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/5] qemu: Add gl property to graphics of type sdl in domain config
On Mon, May 14, 2018 at 08:27:35AM -0400, John Ferlan wrote: On 05/14/2018 07:24 AM, Martin Kletzander wrote: On Fri, May 11, 2018 at 03:09:20PM +0100, Maciej Wolny wrote: On 11/05/18 09:42, Martin Kletzander wrote: On Thu, May 10, 2018 at 11:53:57AM +0100, Maciej Wolny wrote: Support OpenGL accelerated rendering when using SDL graphics in the domain config. Add associated test and documentation. Signed-off-by: Maciej Wolny--- docs/formatdomain.html.in | 6 +++ docs/schemas/domaincommon.rng | 8 src/conf/domain_conf.c | 44 - src/conf/domain_conf.h | 1 + docs, conf and schemas fit together nicely, they should be in one patch, but. tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 ++ .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 ++ tests/qemuxml2xmltest.c | 1 + this has nothing to do with qemu (yet), also see Subject (I wouldn't say 'qemu:' there, but rather something like 'docs, conf, schema:') For the XML tests above you can use genericxml2xmltest instead of the QEMU-specific one. The option only makes sense in QEMU afaik, hence the naming. Yes, for now. If someone who's building the code without QEMU driver changes the behaviour, the tests will pass if you keep it in qemuxml2xml, however genericxml2xml will catch that. qemuxml2xml should be testing specifics where you behave based on some more information than just generic XML. I hope that's clear. Have a nice day. However, until qemuxml2argvtest can also pull out of genericxml2xmldata, then you'd have separate xml input and output files - is that what's desired? Taking a quick look just now - see the graphics-vnc-socket - do we want to duplicate having two input/output XML files which invariably will diverge? Ironically the generic one has a domain type == qemu, an emulator using qemu, and the socket path using QEMU - so while it's generic in one sense, it's not in others. Even more ironic is the qemu specific file uses "". Could/should generification of the xml2xml tests be considered a "bite sized task"? Oh, definitely. It's only some time ago that the tests started to be usable IIRC, so hopefully we'll migrate some XMLs here and there. But maybe others could chime in as well so that I don't speak for others. I remember Pavel having some ideas for cleaner separation of those. signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 6/5] docs/news.xml: Update with QEMU SDL OpenGL Improvement
$SUBJ: docs: Update news.xml with QEMU SDL OpenGL Improvement On 05/11/2018 10:47 AM, Maciej Wolny wrote: > Signed-off-by: Maciej Wolny> --- > docs/news.xml | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/docs/news.xml b/docs/news.xml > index 80181415c..e4a10f667 100644 > --- a/docs/news.xml > +++ b/docs/news.xml > @@ -37,6 +37,15 @@ > > > > + > + > +qemu: Add suport for OpenGL rendering with SDL > + > + > +Domains using SDL as a graphics backend will now be able to use > +OpenGL accelerated rendering. > + > + This should have been put into the 4.4 section not the comments... And each indention level can be 2 spaces... I will move/fix it... Reviewed-by: John Ferlan John > > > > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/5] qemu: Add gl property to graphics of type sdl in domain config
On 05/14/2018 07:24 AM, Martin Kletzander wrote: > On Fri, May 11, 2018 at 03:09:20PM +0100, Maciej Wolny wrote: >> On 11/05/18 09:42, Martin Kletzander wrote: >>> On Thu, May 10, 2018 at 11:53:57AM +0100, Maciej Wolny wrote: Support OpenGL accelerated rendering when using SDL graphics in the domain config. Add associated test and documentation. Signed-off-by: Maciej Wolny--- docs/formatdomain.html.in | 6 +++ docs/schemas/domaincommon.rng | 8 src/conf/domain_conf.c | 44 - src/conf/domain_conf.h | 1 + >>> >>> docs, conf and schemas fit together nicely, they should be in one >>> patch, but. >>> tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 ++ .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 ++ tests/qemuxml2xmltest.c | 1 + >>> >>> this has nothing to do with qemu (yet), also see Subject (I wouldn't say >>> 'qemu:' there, but rather something like 'docs, conf, schema:') >>> >>> For the XML tests above you can use genericxml2xmltest instead of the >>> QEMU-specific one. >> >> The option only makes sense in QEMU afaik, hence the naming. >> > > Yes, for now. If someone who's building the code without QEMU driver > changes > the behaviour, the tests will pass if you keep it in qemuxml2xml, however > genericxml2xml will catch that. qemuxml2xml should be testing specifics > where > you behave based on some more information than just generic XML. > > I hope that's clear. > > Have a nice day. > However, until qemuxml2argvtest can also pull out of genericxml2xmldata, then you'd have separate xml input and output files - is that what's desired? Taking a quick look just now - see the graphics-vnc-socket - do we want to duplicate having two input/output XML files which invariably will diverge? Ironically the generic one has a domain type == qemu, an emulator using qemu, and the socket path using QEMU - so while it's generic in one sense, it's not in others. Even more ironic is the qemu specific file uses "". Could/should generification of the xml2xml tests be considered a "bite sized task"? John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/5] qemu: Add gl property to graphics of type sdl in domain config
On Fri, May 11, 2018 at 03:09:20PM +0100, Maciej Wolny wrote: On 11/05/18 09:42, Martin Kletzander wrote: On Thu, May 10, 2018 at 11:53:57AM +0100, Maciej Wolny wrote: Support OpenGL accelerated rendering when using SDL graphics in the domain config. Add associated test and documentation. Signed-off-by: Maciej Wolny--- docs/formatdomain.html.in | 6 +++ docs/schemas/domaincommon.rng | 8 src/conf/domain_conf.c | 44 - src/conf/domain_conf.h | 1 + docs, conf and schemas fit together nicely, they should be in one patch, but. tests/qemuxml2argvdata/video-virtio-gpu-sdl-gl.xml | 38 ++ .../qemuxml2xmloutdata/video-virtio-gpu-sdl-gl.xml | 45 ++ tests/qemuxml2xmltest.c | 1 + this has nothing to do with qemu (yet), also see Subject (I wouldn't say 'qemu:' there, but rather something like 'docs, conf, schema:') For the XML tests above you can use genericxml2xmltest instead of the QEMU-specific one. The option only makes sense in QEMU afaik, hence the naming. Yes, for now. If someone who's building the code without QEMU driver changes the behaviour, the tests will pass if you keep it in qemuxml2xml, however genericxml2xml will catch that. qemuxml2xml should be testing specifics where you behave based on some more information than just generic XML. I hope that's clear. Have a nice day. signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/5] qemu: Add QEMU_CAPS_SDL_GL to qemu capabilities
On Fri, May 11, 2018 at 03:14:26PM +0100, Maciej Wolny wrote: On 11/05/18 11:34, John Ferlan wrote:> I actually spent some time trying to figure out which magic incantation of the virQEMUCaps* would work. I even tried various forms in virQEMUCapsQMPSchemaQueries, but could not get the flag to be added from qemu 2.4 and beyond. Again, my "assumption" is that it was hand edited into the 2.4 replies that was added here mainly because none of the other available sdl * options were addressed - just the one that was wanted. It was hand-edited. I'm still not sure how this work.. I ran the tool to generate a .replies file; but based on that, how do I get the replies for all historical version of QEMU, do I need to have each of the versions of QEMU installed on my system. Unfortunately, yes. There was some work done so taht we could automate that, but nobody actually finished the work. Anyway, since QEMU does not expose that capability in the QMP communication (what libvirt uses to check for the support), we need to guess it based on the release version :( signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 07/12] Bhyve: Fix command line generation to correctly pick up local loader path.
Signed-off-by: Prerna Saxena--- src/bhyve/bhyve_command.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 9413ae5..2b67014 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -518,10 +518,12 @@ virBhyveProcessBuildBhyveCmd(virConnectPtr conn, virCommandAddArgList(cmd, "-s", "0:0,hostbridge", NULL); if (def->os.bootloader == NULL && -def->os.loader) { +def->os.loader && +def->os.loader->src && +def->os.loader->src->type == VIR_STORAGE_TYPE_FILE) { if ((bhyveDriverGetCaps(conn) & BHYVE_CAP_LPC_BOOTROM)) { virCommandAddArg(cmd, "-l"); -virCommandAddArgFormat(cmd, "bootrom,%s", def->os.loader->path); +virCommandAddArgFormat(cmd, "bootrom,%s", def->os.loader->src->path); add_lpc = true; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 09/12] Vbox: Adjust references to 'loader' and 'nvram' elements given that these are now represented by virStorageSourcePtr.
Signed-off-by: Prerna Saxena--- src/vbox/vbox_common.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 72a24a3..60451a3 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -998,11 +998,16 @@ vboxSetBootDeviceOrder(virDomainDefPtr def, vboxDriverPtr data, VIR_DEBUG("def->os.initrd %s", def->os.initrd); VIR_DEBUG("def->os.cmdline %s", def->os.cmdline); VIR_DEBUG("def->os.root %s", def->os.root); -if (def->os.loader) { -VIR_DEBUG("def->os.loader->path %s", def->os.loader->path); +if (def->os.loader && +def->os.loader->src && +def->os.loader->src->type == VIR_STORAGE_TYPE_FILE) { + +VIR_DEBUG("def->os.loader->src->path %s", def->os.loader->src->path); VIR_DEBUG("def->os.loader->readonly %d", def->os.loader->readonly); VIR_DEBUG("def->os.loader->type %d", def->os.loader->type); -VIR_DEBUG("def->os.loader->nvram%s", def->os.loader->nvram); +if (def->os.loader->nvram && +def->os.loader->nvram->type == VIR_STORAGE_TYPE_FILE) +VIR_DEBUG("def->os.loader->nvram->path%s", def->os.loader->nvram->path); } VIR_DEBUG("def->os.bootloader %s", def->os.bootloader); VIR_DEBUG("def->os.bootloaderArgs %s", def->os.bootloaderArgs); -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 11/12] Test: Add a test snippet to evaluate command line generation for loader/nvram specified via virStorageSource
Signed-off-by: Prerna Saxena--- tests/qemuxml2argvdata/bios-nvram-network.args | 31 +++ tests/qemuxml2argvdata/bios-nvram-network.xml | 42 ++ tests/qemuxml2argvtest.c | 1 + 3 files changed, 74 insertions(+) create mode 100644 tests/qemuxml2argvdata/bios-nvram-network.args create mode 100644 tests/qemuxml2argvdata/bios-nvram-network.xml diff --git a/tests/qemuxml2argvdata/bios-nvram-network.args b/tests/qemuxml2argvdata/bios-nvram-network.args new file mode 100644 index 000..3fc68ef --- /dev/null +++ b/tests/qemuxml2argvdata/bios-nvram-network.args @@ -0,0 +1,31 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name test-bios \ +-S \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-drive file=/usr/share/OVMF/OVMF_CODE.fd,if=pflash,format=raw,unit=0,\ +readonly=on \ +-drive file=iscsi://example.org:6000/iqn.1992-01.com.example/0,\ +if=pflash,format=raw,unit=1 \ +-m 1024 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 362d1fc1-df7d-193e-5c18-49a71bd1da66 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-test-bios/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot order=c,menu=on \ +-usb \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \ +-device usb-tablet,id=input0,bus=usb.0,port=1 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/bios-nvram-network.xml b/tests/qemuxml2argvdata/bios-nvram-network.xml new file mode 100644 index 000..bad114c --- /dev/null +++ b/tests/qemuxml2argvdata/bios-nvram-network.xml @@ -0,0 +1,42 @@ + + test-bios + 362d1fc1-df7d-193e-5c18-49a71bd1da66 + 1048576 + 1048576 + 1 + +hvm + + + + + + + + + + + + + + + + destroy + restart + restart + +/usr/bin/qemu-system-x86_64 + + + + + + + + + + + + + + diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index ddf567b..7338dba 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -853,6 +853,7 @@ mymain(void) QEMU_CAPS_DEVICE_ISA_SERIAL, QEMU_CAPS_SGA); DO_TEST("bios-nvram", NONE); +DO_TEST("bios-nvram-network", NONE); DO_TEST("bios-nvram-secure", QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, QEMU_CAPS_DEVICE_PCI_BRIDGE, -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 05/12] Plumb the loader source into generation of QEMU command line.
Given that nvram & loader elements can now be backed by a non-local source too, adjust all steps leading to generation of QEMU command line. Signed-off-by: Prerna Saxena--- src/qemu/qemu_cgroup.c | 13 + src/qemu/qemu_command.c | 21 - src/qemu/qemu_domain.c | 31 +- src/qemu/qemu_driver.c | 7 --- src/qemu/qemu_process.c | 42 - src/security/security_dac.c | 6 -- src/security/security_selinux.c | 6 -- 7 files changed, 88 insertions(+), 38 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index d88eb78..2068eb0 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -580,16 +580,21 @@ qemuSetupMemoryCgroup(virDomainObjPtr vm) static int qemuSetupFirmwareCgroup(virDomainObjPtr vm) { +virStorageSourcePtr src = NULL; + if (!vm->def->os.loader) return 0; -if (vm->def->os.loader->path && -qemuSetupImagePathCgroup(vm, vm->def->os.loader->path, - vm->def->os.loader->readonly == VIR_TRISTATE_BOOL_YES) < 0) +src = vm->def->os.loader->src; + +if (src->type == VIR_STORAGE_TYPE_FILE && +qemuSetupImagePathCgroup(vm, src->path, + src->readonly == VIR_TRISTATE_BOOL_YES) < 0) return -1; if (vm->def->os.loader->nvram && -qemuSetupImagePathCgroup(vm, vm->def->os.loader->nvram, false) < 0) +vm->def->os.loader->nvram->type == VIR_STORAGE_TYPE_FILE && +qemuSetupImagePathCgroup(vm, vm->def->os.loader->nvram->path, false) < 0) return -1; return 0; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 08f67a4..e9d6e4b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9260,6 +9260,7 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd, virDomainLoaderDefPtr loader = def->os.loader; virBuffer buf = VIR_BUFFER_INITIALIZER; int unit = 0; +char *source = NULL; if (!loader) return; @@ -9267,7 +9268,7 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd, switch ((virDomainLoader) loader->type) { case VIR_DOMAIN_LOADER_TYPE_ROM: virCommandAddArg(cmd, "-bios"); -virCommandAddArg(cmd, loader->path); +virCommandAddArg(cmd, loader->src->path); break; case VIR_DOMAIN_LOADER_TYPE_PFLASH: @@ -9279,9 +9280,14 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd, NULL); } +if (qemuGetDriveSourceString(loader->src, NULL, ) < 0) +break; + virBufferAddLit(, "file="); -virQEMUBuildBufferEscapeComma(, loader->path); -virBufferAsprintf(, ",if=pflash,format=raw,unit=%d", unit); +virQEMUBuildBufferEscapeComma(, source); +free(source); +virBufferAsprintf(, ",if=pflash,format=raw,unit=%d", + unit); unit++; if (loader->readonly) { @@ -9294,9 +9300,14 @@ qemuBuildDomainLoaderCommandLine(virCommandPtr cmd, if (loader->nvram) { virBufferFreeAndReset(); +if (qemuGetDriveSourceString(loader->nvram, NULL, ) < 0) +break; + virBufferAddLit(, "file="); -virQEMUBuildBufferEscapeComma(, loader->nvram); -virBufferAsprintf(, ",if=pflash,format=raw,unit=%d", unit); +virQEMUBuildBufferEscapeComma(, source); +virBufferAsprintf(, ",if=pflash,format=raw,unit=%d", + unit); +unit++; virCommandAddArg(cmd, "-drive"); virCommandAddArgBuffer(cmd, ); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9bb6d8a..2d4e299 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3318,6 +3318,9 @@ qemuDomainDefPostParse(virDomainDefPtr def, * function shall not fail in that case. It will be re-run on VM startup * with the capabilities populated. */ virQEMUCapsPtr qemuCaps = parseOpaque; +virDomainLoaderDefPtr ldr = NULL; +char *nvramPath = NULL; + int ret = -1; if (def->os.bootloader || def->os.bootloaderArgs) { @@ -3332,13 +3335,20 @@ qemuDomainDefPostParse(virDomainDefPtr def, goto cleanup; } -if (def->os.loader && -def->os.loader->type == VIR_DOMAIN_LOADER_TYPE_PFLASH && -def->os.loader->readonly == VIR_TRISTATE_SWITCH_ON && -!def->os.loader->nvram) { -if (virAsprintf(>os.loader->nvram, "%s/%s_VARS.fd", +ldr = def->os.loader; +if (ldr && +ldr->type == VIR_DOMAIN_LOADER_TYPE_PFLASH && +ldr->readonly == VIR_TRISTATE_SWITCH_ON && +!ldr->nvram) { +if (virAsprintf(, "%s/%s_VARS.fd", cfg->nvramDir, def->name) < 0) goto cleanup; +ldr->nvram =
[libvirt] [PATCH 06/12] Fix the domain def inference logic to correctly account for network-backed pflash devices.
Signed-off-by: Prerna Saxena--- src/qemu/qemu_parse_command.c | 30 +++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_parse_command.c b/src/qemu/qemu_parse_command.c index 351425f..9b1a86e 100644 --- a/src/qemu/qemu_parse_command.c +++ b/src/qemu/qemu_parse_command.c @@ -650,6 +650,7 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, int idx = -1; int busid = -1; int unitid = -1; +bool is_firmware = false; if (qemuParseKeywords(val, , @@ -772,6 +773,9 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, def->bus = VIR_DOMAIN_DISK_BUS_VIRTIO; } else if (STREQ(values[i], "xen")) { def->bus = VIR_DOMAIN_DISK_BUS_XEN; +} else if (STREQ(values[i], "pflash")) { +def->bus = VIR_DOMAIN_DISK_BUS_LAST; +is_firmware = true; } else if (STREQ(values[i], "sd")) { def->bus = VIR_DOMAIN_DISK_BUS_SD; } @@ -943,8 +947,25 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, ignore_value(VIR_STRDUP(def->dst, "hda")); } -if (!def->dst) -goto error; +if (!def->dst) { +if (is_firmware && def->bus == VIR_DOMAIN_DISK_BUS_LAST) { +if (!dom->os.loader && (VIR_ALLOC(dom->os.loader) < 0)) +goto error; +if (def->src->readonly) { +/* Loader spec */ +dom->os.loader->src = def->src; +dom->os.loader->type = VIR_DOMAIN_LOADER_TYPE_PFLASH; +} else { +/* NVRAM Spec */ +if (!dom->os.loader->nvram && (VIR_ALLOC(dom->os.loader->nvram) < 0)) +goto error; +dom->os.loader->nvram = def->src; +} +} else { +goto error; +} +} + if (STREQ(def->dst, "xvda")) def->dst[3] = 'a' + idx; else @@ -2215,8 +2236,11 @@ qemuParseCommandLine(virCapsPtr caps, } else if (STREQ(arg, "-bios")) { WANT_VALUE(); if (VIR_ALLOC(def->os.loader) < 0 || -VIR_STRDUP(def->os.loader->path, val) < 0) +VIR_ALLOC(def->os.loader->src) < 0 || +VIR_STRDUP(def->os.loader->src->path, val) < 0) goto error; +def->os.loader->src->type = VIR_STORAGE_TYPE_FILE; +def->os.loader->type = VIR_DOMAIN_LOADER_TYPE_ROM; } else if (STREQ(arg, "-initrd")) { WANT_VALUE(); if (VIR_STRDUP(def->os.initrd, val) < 0) -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 01/12] Schema: Introduce XML schema for network-backed loader and nvram elements.
Today, 'loader' and 'nvram' elements are supposed to be backed by a local disk. Given that NVRAM data could be network-backed for high availability, this patch defines XML spec for serving loader & nvram disks via the network. Signed-off-by: Prerna Saxena--- docs/schemas/domaincommon.rng | 108 +++--- 1 file changed, 90 insertions(+), 18 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 0a6b29b..a6207db 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -276,7 +276,42 @@ - + + + + file + network + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -287,7 +322,40 @@ - + + + file + network + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -1494,25 +1562,29 @@ - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + block -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 10/12] Xen: Adjust all references to loader & nvram elements given that they are now backed by virStorageSourcePtr
Signed-off-by: Prerna Saxena--- src/xenapi/xenapi_driver.c | 4 +++- src/xenconfig/xen_sxpr.c | 19 +++ src/xenconfig/xen_xm.c | 9 ++--- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index 42b305d..4070660 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -1444,10 +1444,12 @@ xenapiDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) char *value = NULL; defPtr->os.type = VIR_DOMAIN_OSTYPE_XEN; if (VIR_ALLOC(defPtr->os.loader) < 0 || -VIR_STRDUP(defPtr->os.loader->path, "pygrub") < 0) { +VIR_ALLOC(defPtr->os.loader->src) < 0 || +VIR_STRDUP(defPtr->os.loader->src->path, "pygrub") < 0) { VIR_FREE(boot_policy); goto error; } +defPtr->os.loader->src->type = VIR_STORAGE_TYPE_FILE; xen_vm_get_pv_kernel(session, , vm); if (STRNEQ(value, "")) { if (VIR_STRDUP(defPtr->os.kernel, value) < 0) { diff --git a/src/xenconfig/xen_sxpr.c b/src/xenconfig/xen_sxpr.c index e868c05..fd3165c 100644 --- a/src/xenconfig/xen_sxpr.c +++ b/src/xenconfig/xen_sxpr.c @@ -87,15 +87,17 @@ xenParseSxprOS(const struct sexpr *node, int hvm) { if (hvm) { -if (VIR_ALLOC(def->os.loader) < 0) +if ((VIR_ALLOC(def->os.loader) < 0) || +(VIR_ALLOC(def->os.loader->src) < 0)) goto error; -if (sexpr_node_copy(node, "domain/image/hvm/loader", >os.loader->path) < 0) +def->os.loader->src->type = VIR_STORAGE_TYPE_FILE; +if (sexpr_node_copy(node, "domain/image/hvm/loader", >os.loader->src->path) < 0) goto error; -if (def->os.loader->path == NULL) { -if (sexpr_node_copy(node, "domain/image/hvm/kernel", >os.loader->path) < 0) +if (def->os.loader->src->path == NULL) { +if (sexpr_node_copy(node, "domain/image/hvm/kernel", >os.loader->src->path) < 0) goto error; -if (def->os.loader->path == NULL) { +if (def->os.loader->src->path == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("domain information incomplete, missing HVM loader")); return -1; @@ -124,7 +126,8 @@ xenParseSxprOS(const struct sexpr *node, /* If HVM kenrel == loader, then old xend, so kill off kernel */ if (hvm && def->os.kernel && -STREQ(def->os.kernel, def->os.loader->path)) { +def->os.loader->src && +STREQ(def->os.kernel, def->os.loader->src->path)) { VIR_FREE(def->os.kernel); } /* Drop kernel argument that has no value */ @@ -2259,9 +2262,9 @@ xenFormatSxpr(virConnectPtr conn, virDomainDefPtr def) if (hvm) { char bootorder[VIR_DOMAIN_BOOT_LAST+1]; if (def->os.kernel) -virBufferEscapeSexpr(, "(loader '%s')", def->os.loader->path); +virBufferEscapeSexpr(, "(loader '%s')", def->os.loader->src->path); else -virBufferEscapeSexpr(, "(kernel '%s')", def->os.loader->path); +virBufferEscapeSexpr(, "(kernel '%s')", def->os.loader->src->path); virBufferAsprintf(, "(vcpus %u)", virDomainDefGetVcpusMax(def)); if (virDomainDefHasVcpusOffline(def)) diff --git a/src/xenconfig/xen_xm.c b/src/xenconfig/xen_xm.c index 8ef68bb..c6a1f03 100644 --- a/src/xenconfig/xen_xm.c +++ b/src/xenconfig/xen_xm.c @@ -47,8 +47,10 @@ xenParseXMOS(virConfPtr conf, virDomainDefPtr def) const char *boot; if (VIR_ALLOC(def->os.loader) < 0 || -xenConfigCopyString(conf, "kernel", >os.loader->path) < 0) +VIR_ALLOC(def->os.loader->src) < 0 || +xenConfigCopyString(conf, "kernel", >os.loader->src->path) < 0) return -1; +def->os.loader->src->type = VIR_STORAGE_TYPE_FILE; if (xenConfigGetString(conf, "boot", , "c") < 0) return -1; @@ -481,9 +483,10 @@ xenFormatXMOS(virConfPtr conf, virDomainDefPtr def) if (xenConfigSetString(conf, "builder", "hvm") < 0) return -1; -if (def->os.loader && def->os.loader->path && -xenConfigSetString(conf, "kernel", def->os.loader->path) < 0) +if (def->os.loader && def->os.loader->src && +xenConfigSetString(conf, "kernel", def->os.loader->src->path) < 0) return -1; +def->os.loader->src->type = VIR_STORAGE_TYPE_FILE; for (i = 0; i < def->os.nBootDevs; i++) { switch (def->os.bootDevs[i]) { -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/12] Format the loader source appropriately.
If the initial XML used the old-style declaration as follows: /path/to/file we format it as was read. However, if it used new-style declaration: The formatter identifies that this is a new-style format and renders it likewise. Signed-off-by: Prerna Saxena--- src/conf/domain_conf.c | 86 -- 1 file changed, 76 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index be43695..d59a579 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -18094,7 +18094,7 @@ virDomainLoaderNvramDefParseXML(xmlNodePtr node, } loader->nvram->type = VIR_STORAGE_TYPE_LAST; -loader->nvram->oldStyleNvram = false; +loader->oldStyleNvram = false; if ((tmp = virXMLPropString(node, "backing")) && (loader->nvram->type = virStorageTypeFromString(tmp)) <= 0) { @@ -26212,11 +26212,19 @@ virDomainHugepagesFormat(virBufferPtr buf, static void virDomainLoaderDefFormat(virBufferPtr buf, - virDomainLoaderDefPtr loader) + virDomainLoaderDefPtr loader, + unsigned int flags) { const char *readonly = virTristateBoolTypeToString(loader->readonly); const char *secure = virTristateBoolTypeToString(loader->secure); const char *type = virDomainLoaderTypeToString(loader->type); +const char *backing = NULL; + +virBuffer attrBuf = VIR_BUFFER_INITIALIZER; +virBuffer childBuf = VIR_BUFFER_INITIALIZER; + +virBufferSetChildIndent(, buf); + virBufferAddLit(buf, " secure) virBufferAsprintf(buf, " secure='%s'", secure); -virBufferAsprintf(buf, " type='%s'>", type); +virBufferAsprintf(buf, " type='%s'", type); +if (loader->src && +loader->src->type < VIR_STORAGE_TYPE_LAST) { +if (!loader->oldStyleLoader) { +/* Format this in the new style, using the + * sub-element */ +if (virDomainStorageSourceFormat(, , loader->src, + flags, 0) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot format loader source")); +goto cleanup; +} + +backing = virStorageTypeToString(loader->src->type); +virBufferAsprintf(buf, " backing='%s'>", backing); + +if (virXMLFormatElement(buf, "source", , ) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot format loader source")); +goto cleanup; +} + +} else +/* Format this in the old-style, using absolute paths directly. */ +virBufferAsprintf(buf, ">%s", loader->src->path); +} else +virBufferAddLit(buf, ">\n"); + +virBufferAddLit(buf, "\n"); -virBufferEscapeString(buf, "%s\n", loader->path); if (loader->nvram || loader->templt) { +ignore_value(virBufferContentAndReset()); +ignore_value(virBufferContentAndReset()); +virBufferSetChildIndent(, buf); + virBufferAddLit(buf, " templt); -if (loader->nvram) -virBufferEscapeString(buf, ">%s\n", loader->nvram); -else -virBufferAddLit(buf, "/>\n"); + +if (loader->templt) +virBufferEscapeString(buf, " template='%s'", loader->templt); + +if (loader->nvram) { +backing = virStorageTypeToString(loader->nvram->type); +if (!loader->oldStyleNvram) { +if (virDomainStorageSourceFormat(, , + loader->nvram, flags, 0) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot format NVRAM source")); +virBufferAddLit(buf, ">\n\n"); +goto cleanup; +} + +virBufferEscapeString(buf, " backing='%s'>", backing); +if (virXMLFormatElement(buf, "source", , ) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot format NVRAM source")); +virBufferAddLit(buf, "\n"); +goto cleanup; +} +} else { +/* old-style NVRAM declaration found */ +virBufferAsprintf(buf, ">%s", loader->nvram->path); +} +virBufferAddLit(buf, "\n\n"); +} } +cleanup: +virBufferFreeAndReset(); +virBufferFreeAndReset(); +return; } static void @@ -26899,7 +26965,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAsprintf(buf, "%s\n", def->os.initgroup); if (def->os.loader) -
[libvirt] [PATCH 08/12] virt-aa-helper: Adjust references to loader & nvram elements to correctly parse the virStorageSource types.
Signed-off-by: Prerna Saxena--- src/security/virt-aa-helper.c | 14 ++ 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index d0f9876..8217d67 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1063,12 +1063,18 @@ get_files(vahControl * ctl) if (vah_add_file(, ctl->def->os.slic_table, "r") != 0) goto cleanup; -if (ctl->def->os.loader && ctl->def->os.loader->path) -if (vah_add_file(, ctl->def->os.loader->path, "rk") != 0) +if (ctl->def->os.loader && +ctl->def->os.loader->src && +ctl->def->os.loader->src->type == VIR_STORAGE_TYPE_FILE && +ctl->def->os.loader->src->path) +if (vah_add_file(, ctl->def->os.loader->src->path, "rk") != 0) goto cleanup; -if (ctl->def->os.loader && ctl->def->os.loader->nvram) -if (vah_add_file(, ctl->def->os.loader->nvram, "rwk") != 0) +if (ctl->def->os.loader && +ctl->def->os.loader->nvram && +ctl->def->os.loader->nvram->type == VIR_STORAGE_TYPE_FILE && +ctl->def->os.loader->nvram->path) +if (vah_add_file(, ctl->def->os.loader->nvram->path, "rwk") != 0) goto cleanup; for (i = 0; i < ctl->def->ngraphics; i++) { -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 00/12][v2] Introduce network-backed loader & nvram.
Libvirt domain XML today only allows local filepaths that can be used to specify a loader element or its matching NVRAM disk. Given that Vms may themselves move across hypervisor hosts, it should be possible to allocate loaders/NVRAM disks on network storage for uninterrupted access. This series extends the firmware loader & NVRAM disks to be hosted over network-backed disks, for high availability. It achieves this by embedding virStorageSource elements for loader & nvram into _virDomainLoaderDef, as discussed in https://www.redhat.com/archives/libvir-list/2018-March/msg01721.html. Currently, the source type is annotated by introducing a new attribute "backing" for both 'loader' and 'nvram' elements. Hence, a sample XML with new annotation looks like this: References: -- v0 / Proposal: https://www.redhat.com/archives/libvir-list/2018-March/msg01721.html. v1 : https://www.redhat.com/archives/libvir-list/2018-April/msg02024.html Changelog: - Changes since v1: 1. Split up the patch into smaller units. 2. Added doc snippet & tests. 3. Changed the formatting code in patch 4 to format a domain's XML in the style that was read. I found that encryption seems to be a property of the storage volume. I didnt include that in this series since it does not use backing type as volume. Will include that later once the basic network support patches get done. Looking forward to feedback, Prerna Prerna Saxena (12): Schema: Introduce XML schema for network-backed loader and nvram elements. Loader: Add a more elaborate definition. Parse domain XML to generate virDomainLoaderDef & virDomainNvramDef. Format the loader source appropriately. Plumb the loader source into generation of QEMU command line. Fix the domain def inference logic to correctly account for network-backed pflash devices. Bhyve: Fix command line generation to correctly pick up local loader path. virt-aa-helper: Adjust references to loader & nvram elements to correctly parse the virStorageSource types. Vbox: Adjust references to 'loader' and 'nvram' elements given that these are now represented by virStorageSourcePtr. Xen: Adjust all references to loader & nvram elements given that they are now backed by virStorageSourcePtr Test: Add a test snippet to evaluate command line generation for loader/nvram specified via virStorageSource Documentation: Add a blurb for the newly added XML snippets for loader and nvram. docs/formatdomain.html.in | 36 - docs/schemas/domaincommon.rng | 108 ++--- src/bhyve/bhyve_command.c | 6 +- src/conf/domain_conf.c | 215 +++-- src/conf/domain_conf.h | 11 +- src/qemu/qemu_cgroup.c | 13 +- src/qemu/qemu_command.c| 21 ++- src/qemu/qemu_domain.c | 31 ++-- src/qemu/qemu_driver.c | 7 +- src/qemu/qemu_parse_command.c | 30 +++- src/qemu/qemu_process.c| 42 +++-- src/security/security_dac.c| 6 +- src/security/security_selinux.c| 6 +- src/security/virt-aa-helper.c | 14 +- src/vbox/vbox_common.c | 11 +- src/xenapi/xenapi_driver.c | 4 +- src/xenconfig/xen_sxpr.c | 19 ++- src/xenconfig/xen_xm.c | 9 +- tests/qemuxml2argvdata/bios-nvram-network.args | 31 tests/qemuxml2argvdata/bios-nvram-network.xml | 42 + tests/qemuxml2argvtest.c | 1 + 21 files changed, 562 insertions(+), 101 deletions(-) create mode 100644 tests/qemuxml2argvdata/bios-nvram-network.args create mode 100644 tests/qemuxml2argvdata/bios-nvram-network.xml -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/12] Parse domain XML to generate virDomainLoaderDef & virDomainNvramDef.
This patch is used to interpret domain XML and store the Loader & Nvram's backing definitions as virStorageSource. It also identifies if input XML used old or new-style declaration. (This will later be used by the formatter). Signed-off-by: Prerna Saxena--- src/conf/domain_conf.c | 131 ++--- 1 file changed, 124 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f678e26..be43695 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2884,8 +2884,8 @@ virDomainLoaderDefFree(virDomainLoaderDefPtr loader) if (!loader) return; -VIR_FREE(loader->path); -VIR_FREE(loader->nvram); +virStorageSourceFree(loader->src); +virStorageSourceFree(loader->nvram); VIR_FREE(loader->templt); VIR_FREE(loader); } @@ -17986,17 +17986,62 @@ virDomainDefMaybeAddHostdevSCSIcontroller(virDomainDefPtr def) static int virDomainLoaderDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, virDomainLoaderDefPtr loader) { int ret = -1; char *readonly_str = NULL; char *secure_str = NULL; char *type_str = NULL; +char *tmp = NULL; +xmlNodePtr cur; +xmlXPathContextPtr cur_ctxt = ctxt; + +if (VIR_ALLOC(loader->src)) { +goto cleanup; +} +loader->src->type = VIR_STORAGE_TYPE_LAST; +loader->oldStyleLoader = false; readonly_str = virXMLPropString(node, "readonly"); secure_str = virXMLPropString(node, "secure"); type_str = virXMLPropString(node, "type"); -loader->path = (char *) xmlNodeGetContent(node); + +if ((tmp = virXMLPropString(node, "backing")) && +(loader->src->type = virStorageTypeFromString(tmp)) <= 0) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown loader type '%s'"), tmp); +goto cleanup; +} +VIR_FREE(tmp); + +for (cur = node->children; cur != NULL; cur = cur->next) { +if (cur->type != XML_ELEMENT_NODE) { +continue; +} + +if (virXMLNodeNameEqual(cur, "source")) { +/* new-style declaration found */ +if (virDomainStorageSourceParse(cur, cur_ctxt, loader->src, 0) < 0) { +virReportError(VIR_ERR_XML_DETAIL, + _("Error parsing Loader source element")); +goto cleanup; +} +break; +} +} + +/* Old-style absolute path found ? */ +if (loader->src->type == VIR_STORAGE_TYPE_LAST) { +if (!(loader->src->path = (char *) xmlNodeGetContent(node))) { +virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing loader source")); +goto cleanup; +} else { +loader->src->type = VIR_STORAGE_TYPE_FILE; +loader->oldStyleLoader = true; +} +} if (readonly_str && (loader->readonly = virTristateBoolTypeFromString(readonly_str)) <= 0) { @@ -18023,13 +18068,81 @@ virDomainLoaderDefParseXML(xmlNodePtr node, } ret = 0; - cleanup: +goto exit; +cleanup: +if (loader->src) + VIR_FREE(loader->src); +exit: VIR_FREE(readonly_str); VIR_FREE(secure_str); VIR_FREE(type_str); + return ret; } +static int +virDomainLoaderNvramDefParseXML(xmlNodePtr node, + xmlXPathContextPtr ctxt, + virDomainLoaderDefPtr loader) +{ +int ret = -1; +char *tmp = NULL; +xmlNodePtr cur; + +if (VIR_ALLOC(loader->nvram)) { +goto cleanup; +} + +loader->nvram->type = VIR_STORAGE_TYPE_LAST; +loader->nvram->oldStyleNvram = false; + +if ((tmp = virXMLPropString(node, "backing")) && +(loader->nvram->type = virStorageTypeFromString(tmp)) <= 0) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown nvram type '%s'"), tmp); +goto cleanup; +} +VIR_FREE(tmp); + +for (cur = node->children; cur != NULL; cur = cur->next) { +if (cur->type != XML_ELEMENT_NODE) { +continue; +} + +if (virXMLNodeNameEqual(cur, "template")) { +loader->templt = virXPathString("string(./os/nvram[1]/@template)", ctxt); +continue; +} + +if (virXMLNodeNameEqual(cur, "source")) { +if (virDomainStorageSourceParse(cur, ctxt, loader->nvram, 0) < 0) { +virReportError(VIR_ERR_XML_DETAIL, + _("Error parsing nvram source element")); +goto cleanup; +} +ret = 0; +} +} + +/* Old-style declaration found */ +if (loader->nvram->type == VIR_STORAGE_TYPE_LAST) { +if (!(loader->nvram->path = (char *) xmlNodeGetContent(node))) { +virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing
[libvirt] [PATCH 12/12] Documentation: Add a blurb for the newly added XML snippets for loader and nvram.
Signed-off-by: Prerna Saxena--- docs/formatdomain.html.in | 36 +--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index caeb14e..b8cb7ac 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -112,6 +112,20 @@ /os ... +Where loader/NVRAM can also be described as: + + +... + loader readonly='yes' secure='no' type='rom' backing='file' +source file='/usr/share/OVMF/OVMF_CODE.fd'/ + /loader + nvram backing='network' template='/usr/share/OVMF/OVMF_VARS.fd' +source protocol='iscsi' name='iqn.2013-07.com.example:iscsi-nopool/2' + host name='example.com' port='3260'/ +/source + /nvram +... + type The content of the type element specifies the @@ -143,7 +157,15 @@ pflash. Moreover, some firmwares may implement the Secure boot feature. Attribute secure can be used then to control it. -Since 2.1.0 +Since 2.1.0Since 4.5.0 +Loader element can also be specified as a remote disk. The attribute +backing (accepted values are +file and network)can be used to represent +local absolute paths as well as network-backed disks. The details of +backing file may be specified as storage source +Allowed backing type file replaces the old +specification and extends to all hypervisors that supported it, while +type network is only supported by QEMU. nvram Some UEFI firmwares may want to use a non-volatile memory to store some variables. In the host, this is represented as a file and the @@ -154,7 +176,15 @@ from the config file. Note, that for transient domains if the NVRAM file has been created by libvirt it is left behind and it is management application's responsibility to save and remove file (if needed to be -persistent). Since 1.2.8 +persistent). Since 1.2.8 +Since 4.5.0, attribute +backing(accepted values file +and network)can be used to specify a +storage source +of type file or network. The value filedescribes +absolute NVRAM paths and extends the present specification +for all hypervisors that support this, while +network applies only to QEMU. boot The dev attribute takes one of the values "fd", "hd", "cdrom" or "network" and is used to specify the next boot device @@ -2707,7 +2737,7 @@ - source + Disk source Representation of the disk source depends on the disk type attribute value as follows: -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 02/12] Loader: Add a more elaborate definition.
Augment definition to include virStorageSourcePtr that more comprehensively describes the nature of backing element. Also include flags for annotating if input XML definition is old-style or new-style. Signed-off-by: Prerna Saxena--- src/conf/domain_conf.h | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 15d228b..bbd021b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1855,15 +1855,22 @@ typedef enum { VIR_ENUM_DECL(virDomainLoader) +struct _virStorageSource; +typedef struct _virStorageSource* virStorageSourcePtr; + typedef struct _virDomainLoaderDef virDomainLoaderDef; typedef virDomainLoaderDef *virDomainLoaderDefPtr; struct _virDomainLoaderDef { -char *path; +virStorageSourcePtr src; int readonly; /* enum virTristateBool */ virDomainLoader type; int secure; /* enum virTristateBool */ -char *nvram;/* path to non-volatile RAM */ +virStorageSourcePtr nvram; /* path to non-voliatile RAM */ char *templt; /* user override of path to master nvram */ +bool oldStyleLoader; /* is this an old-style XML formatting, + * ie, absolute path is directly specified? */ +bool oldStyleNvram; /* is this an old-style XML formatting, + * ie, absolute path is directly specified? */ }; void virDomainLoaderDefFree(virDomainLoaderDefPtr loader); -- 1.8.1.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 12/13] util: storage: Store PR manager alias in the definition
Rather than always re-generating the alias store it in the definition and in the status XML. Signed-off-by: Peter Krempa--- src/qemu/qemu_command.c | 23 +++-- src/qemu/qemu_command.h | 3 +-- src/qemu/qemu_domain.c| 16 +-- src/qemu/qemu_hotplug.c | 34 ++- src/util/virstoragefile.c | 1 + src/util/virstoragefile.h | 3 +++ tests/qemustatusxml2xmldata/modern-in.xml | 4 7 files changed, 37 insertions(+), 47 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c38dde5a60..84d7d51c7c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9669,7 +9669,6 @@ qemuBuildPanicCommandLine(virCommandPtr cmd, * qemuBuildPRManagerInfoProps: * @disk: disk definition * @propsret: Returns JSON object containing properties of the pr-manager-helper object - * @aliasret: alias of the pr-manager-helper object * * Build the JSON properties for the pr-manager object. * @@ -9678,32 +9677,19 @@ qemuBuildPanicCommandLine(virCommandPtr cmd, */ int qemuBuildPRManagerInfoProps(const virDomainDiskDef *disk, -virJSONValuePtr *propsret, -char **aliasret) +virJSONValuePtr *propsret) { -char *alias = NULL; int ret = -1; *propsret = NULL; -*aliasret = NULL; - -if (virStoragePRDefIsManaged(disk->src->pr)) { -if (VIR_STRDUP(alias, qemuDomainGetManagedPRAlias()) < 0) -goto cleanup; -} else { -if (!(alias = qemuDomainGetUnmanagedPRAlias(disk->info.alias))) -goto cleanup; -} if (virJSONValueObjectCreate(propsret, "s:path", disk->src->pr->path, NULL) < 0) goto cleanup; -VIR_STEAL_PTR(*aliasret, alias); ret = 0; cleanup: -VIR_FREE(alias); return ret; } @@ -9715,7 +9701,6 @@ qemuBuildMasterPRCommandLine(virCommandPtr cmd, size_t i; bool managedAdded = false; virJSONValuePtr props = NULL; -char *alias = NULL; char *tmp = NULL; int ret = -1; @@ -9732,14 +9717,13 @@ qemuBuildMasterPRCommandLine(virCommandPtr cmd, managedAdded = true; } -if (qemuBuildPRManagerInfoProps(disk, , ) < 0) +if (qemuBuildPRManagerInfoProps(disk, ) < 0) goto cleanup; if (!(tmp = virQEMUBuildObjectCommandlineFromJSON("pr-manager-helper", - alias, + disk->src->pr->mgralias, props))) goto cleanup; -VIR_FREE(alias); virJSONValueFree(props); props = NULL; @@ -9749,7 +9733,6 @@ qemuBuildMasterPRCommandLine(virCommandPtr cmd, ret = 0; cleanup: -VIR_FREE(alias); virJSONValueFree(props); return ret; } diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 621592cd79..28bc33558b 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -56,8 +56,7 @@ virCommandPtr qemuBuildCommandLine(virQEMUDriverPtr driver, /* Generate the object properties for pr-manager */ int qemuBuildPRManagerInfoProps(const virDomainDiskDef *disk, -virJSONValuePtr *propsret, -char **alias); +virJSONValuePtr *propsret); /* Generate the object properties for a secret */ int qemuBuildSecretInfoProps(qemuDomainSecretInfoPtr secinfo, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 92217e66fe..1572ce5c2d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1941,6 +1941,9 @@ qemuStorageSourcePrivateDataParse(xmlXPathContextPtr ctxt, src->nodestorage = virXPathString("string(./nodenames/nodename[@type='storage']/@name)", ctxt); src->nodeformat = virXPathString("string(./nodenames/nodename[@type='format']/@name)", ctxt); +if (src->pr) +src->pr->mgralias = virXPathString("string(./reservations/@mgralias)", ctxt); + if (virStorageSourcePrivateDataParseRelPath(ctxt, src) < 0) return -1; @@ -1961,6 +1964,9 @@ qemuStorageSourcePrivateDataFormat(virStorageSourcePtr src, virBufferAddLit(buf, "\n"); } +if (src->pr) +virBufferAsprintf(buf, "\n", src->pr->mgralias); + if (virStorageSourcePrivateDataFormatRelPath(src, buf) < 0) return -1; @@ -11932,7 +11938,8 @@ qemuDomainPrepareDiskCachemode(virDomainDiskDefPtr disk) static int qemuDomainPrepareStorageSourcePR(virStorageSourcePtr src, - qemuDomainObjPrivatePtr priv) + qemuDomainObjPrivatePtr priv, + const
[libvirt] [PATCH 10/13] qemu: command: Move check whether PR manager object props need to be built
Move it out of the formatter function and let the caller decide this. Signed-off-by: Peter Krempa--- src/qemu/qemu_command.c | 9 +++-- src/qemu/qemu_hotplug.c | 3 +++ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7df9979cb2..c38dde5a60 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9687,9 +9687,6 @@ qemuBuildPRManagerInfoProps(const virDomainDiskDef *disk, *propsret = NULL; *aliasret = NULL; -if (!disk->src->pr) -return 0; - if (virStoragePRDefIsManaged(disk->src->pr)) { if (VIR_STRDUP(alias, qemuDomainGetManagedPRAlias()) < 0) goto cleanup; @@ -9725,6 +9722,9 @@ qemuBuildMasterPRCommandLine(virCommandPtr cmd, for (i = 0; i < def->ndisks; i++) { const virDomainDiskDef *disk = def->disks[i]; +if (!disk->src->pr) +continue; + if (virStoragePRDefIsManaged(disk->src->pr)) { if (managedAdded) continue; @@ -9735,9 +9735,6 @@ qemuBuildMasterPRCommandLine(virCommandPtr cmd, if (qemuBuildPRManagerInfoProps(disk, , ) < 0) goto cleanup; -if (!props) -continue; - if (!(tmp = virQEMUBuildObjectCommandlineFromJSON("pr-manager-helper", alias, props))) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 3a26876c10..6557711ec1 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -395,6 +395,9 @@ qemuMaybeBuildPRManagerInfoProps(virDomainObjPtr vm, *propsret = NULL; *aliasret = NULL; +if (!disk->src->pr) +return 0; + if (virStoragePRDefIsManaged(disk->src->pr) && priv->prDaemonRunning) { /* @disk requires qemu-pr-helper but there's already one running. */ -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 13/13] qemu: hotplug: Replace qemuDomainDiskNeedRemovePR
The function can be replaced by much simpler logic. Signed-off-by: Peter Krempa--- src/qemu/qemu_hotplug.c | 44 +++- 1 file changed, 3 insertions(+), 41 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 9481123c19..96042bc06c 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3826,42 +3826,6 @@ static bool qemuIsMultiFunctionDevice(virDomainDefPtr def, } -static int -qemuDomainDiskNeedRemovePR(virDomainObjPtr vm, - virDomainDiskDefPtr disk, - bool *stopDaemon) -{ -qemuDomainObjPrivatePtr priv = vm->privateData; -size_t i; - -*stopDaemon = false; - -if (!disk->src->pr) -return 0; - -if (!virStoragePRDefIsManaged(disk->src->pr)) -return 0; - -for (i = 0; i < vm->def->ndisks; i++) { -const virDomainDiskDef *domainDisk = vm->def->disks[i]; - -if (domainDisk == disk) -continue; - -if (virStoragePRDefIsManaged(domainDisk->src->pr)) -break; -} - -if (i != vm->def->ndisks) -return 0; - -if (priv->prDaemonRunning) -*stopDaemon = true; - -return 0; -} - - static int qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -3875,7 +3839,6 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, char *drivestr; char *objAlias = NULL; char *encAlias = NULL; -bool stopPRDaemon = false; VIR_DEBUG("Removing disk %s from domain %p %s", disk->info.alias, vm, vm->def->name); @@ -3913,9 +3876,6 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, } } -if (qemuDomainDiskNeedRemovePR(vm, disk, ) < 0) -return -1; - qemuDomainObjEnterMonitor(driver, vm); qemuMonitorDriveDel(priv->mon, drivestr); @@ -3953,7 +3913,9 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, } } -if (stopPRDaemon) +/* check if the last disk with managed PR was just removed */ +if (priv->prDaemonRunning && +!virDomainDefHasManagedPR(vm->def)) qemuProcessKillManagedPRDaemon(vm); qemuDomainReleaseDeviceAddress(vm, >info, src); -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 11/13] conf: domain: Add helper to check whether a domain def requires use of PR
Extract the lookup code so that it can be reused later. Signed-off-by: Peter Krempa--- src/conf/domain_conf.c | 21 + src/conf/domain_conf.h | 3 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_process.c | 23 ++- 4 files changed, 27 insertions(+), 21 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 86229db654..6f16e4ade4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -29528,3 +29528,24 @@ virDomainDiskGetDetectZeroesMode(virDomainDiskDiscard discard, return detect_zeroes; } + + +/** + * virDomainDefHasManagedPR: + * @def: domain definition + * + * Returns true if any of the domain disks requires the use of the managed + * persistent reservations infrastructure. + */ +bool +virDomainDefHasManagedPR(const virDomainDef *def) +{ +size_t i; + +for (i = 0; i < def->ndisks; i++) { +if (virStoragePRDefIsManaged(def->disks[i]->src->pr)) +return true; +} + +return false; +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 07d04fb2f9..f1add06155 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3543,4 +3543,7 @@ int virDomainDiskGetDetectZeroesMode(virDomainDiskDiscard discard, virDomainDiskDetectZeroes detect_zeroes); +bool +virDomainDefHasManagedPR(const virDomainDef *def); + #endif /* __DOMAIN_CONF_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index dd10be9753..a0b78f72ba 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -275,6 +275,7 @@ virDomainDefGetVcpus; virDomainDefGetVcpusMax; virDomainDefGetVcpusTopology; virDomainDefHasDeviceAddress; +virDomainDefHasManagedPR; virDomainDefHasMemballoon; virDomainDefHasMemoryHotplug; virDomainDefHasUSB; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index af29bcc59e..5b73a61962 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2748,26 +2748,6 @@ qemuProcessStartManagedPRDaemon(virDomainObjPtr vm) } -static int -qemuProcessMaybeStartManagedPRDaemon(virDomainObjPtr vm) -{ -bool hasManaged = false; -size_t i; - -for (i = 0; i < vm->def->ndisks; i++) { -if (virStoragePRDefIsManaged(vm->def->disks[i]->src->pr)) { -hasManaged = true; -break; -} -} - -if (!hasManaged) -return 0; - -return qemuProcessStartManagedPRDaemon(vm); -} - - static int qemuProcessInitPasswords(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -6285,7 +6265,8 @@ qemuProcessLaunch(virConnectPtr conn, goto cleanup; VIR_DEBUG("Setting up managed PR daemon"); -if (qemuProcessMaybeStartManagedPRDaemon(vm) < 0) +if (virDomainDefHasManagedPR(vm->def) && +qemuProcessStartManagedPRDaemon(vm) < 0) goto cleanup; VIR_DEBUG("Setting domain security labels"); -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 09/13] qemu: process: Change semantics of functions starting PR daemon
Libvirt only manages one PR daemon. This means that we don't need to pass the 'disk' object and also rename the functions dealing with this so that it's obvious we only deal with the managed PR daemon. Signed-off-by: Peter Krempa--- src/qemu/qemu_hotplug.c | 6 +++--- src/qemu/qemu_process.c | 37 - src/qemu/qemu_process.h | 5 ++--- 3 files changed, 21 insertions(+), 27 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 77d37e5ef6..3a26876c10 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -377,7 +377,7 @@ qemuDomainMaybeStartPRDaemon(virDomainObjPtr vm, /* @disk requires qemu-pr-helper but none is running. * Start it now. */ -if (qemuProcessStartPRDaemon(vm, disk) < 0) +if (qemuProcessStartManagedPRDaemon(vm) < 0) return -1; return 1; @@ -567,7 +567,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, qemuDomainDelDiskSrcTLSObject(driver, vm, disk->src); ignore_value(qemuHotplugPrepareDiskAccess(driver, vm, disk, NULL, true)); if (prdStarted) -qemuProcessKillPRDaemon(vm); +qemuProcessKillManagedPRDaemon(vm); goto cleanup; } @@ -3963,7 +3963,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, } if (stopPRDaemon) -qemuProcessKillPRDaemon(vm); +qemuProcessKillManagedPRDaemon(vm); qemuDomainReleaseDeviceAddress(vm, >info, src); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 42b91b39ac..af29bcc59e 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2566,7 +2566,7 @@ qemuProcessBuildPRHelperPidfilePath(virDomainObjPtr vm) void -qemuProcessKillPRDaemon(virDomainObjPtr vm) +qemuProcessKillManagedPRDaemon(virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; virErrorPtr orig_err; @@ -2624,8 +2624,7 @@ qemuProcessStartPRDaemonHook(void *opaque) int -qemuProcessStartPRDaemon(virDomainObjPtr vm, - const virDomainDiskDef *disk) +qemuProcessStartManagedPRDaemon(virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; virQEMUDriverPtr driver = priv->driver; @@ -2640,10 +2639,6 @@ qemuProcessStartPRDaemon(virDomainObjPtr vm, const unsigned long long timeout = 50; /* ms */ int ret = -1; -if (!virStoragePRDefIsManaged(disk->src->pr) || -priv->prDaemonRunning) -return 0; - cfg = virQEMUDriverGetConfig(driver); if (!virFileIsExecutable(cfg->prHelperName)) { @@ -2734,7 +2729,7 @@ qemuProcessStartPRDaemon(virDomainObjPtr vm, goto cleanup; priv->prDaemonRunning = true; -ret = 1; +ret = 0; cleanup: if (ret < 0) { virCommandAbort(cmd); @@ -2754,22 +2749,22 @@ qemuProcessStartPRDaemon(virDomainObjPtr vm, static int -qemuProcessMaybeStartPRDaemon(virDomainObjPtr vm) +qemuProcessMaybeStartManagedPRDaemon(virDomainObjPtr vm) { +bool hasManaged = false; size_t i; -int rv; for (i = 0; i < vm->def->ndisks; i++) { -const virDomainDiskDef *disk = vm->def->disks[i]; - -if ((rv = qemuProcessStartPRDaemon(vm, disk)) < 0) -return -1; - -if (rv > 0) -return 1; +if (virStoragePRDefIsManaged(vm->def->disks[i]->src->pr)) { +hasManaged = true; +break; +} } -return 0; +if (!hasManaged) +return 0; + +return qemuProcessStartManagedPRDaemon(vm); } @@ -6289,8 +6284,8 @@ qemuProcessLaunch(virConnectPtr conn, if (qemuProcessResctrlCreate(driver, vm) < 0) goto cleanup; -VIR_DEBUG("Setting up PR daemon"); -if (qemuProcessMaybeStartPRDaemon(vm) < 0) +VIR_DEBUG("Setting up managed PR daemon"); +if (qemuProcessMaybeStartManagedPRDaemon(vm) < 0) goto cleanup; VIR_DEBUG("Setting domain security labels"); @@ -6821,7 +6816,7 @@ void qemuProcessStop(virQEMUDriverPtr driver, qemuDomainMasterKeyRemove(priv); /* Do this before we delete the tree and remove pidfile. */ -qemuProcessKillPRDaemon(vm); +qemuProcessKillManagedPRDaemon(vm); virFileDeleteTree(priv->libDir); virFileDeleteTree(priv->channelTargetDir); diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index eda6695415..a0e34b1c85 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -205,9 +205,8 @@ int qemuProcessRefreshDisks(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainAsyncJob asyncJob); -int qemuProcessStartPRDaemon(virDomainObjPtr vm, - const virDomainDiskDef *disk); +int qemuProcessStartManagedPRDaemon(virDomainObjPtr vm); -void qemuProcessKillPRDaemon(virDomainObjPtr vm); +void qemuProcessKillManagedPRDaemon(virDomainObjPtr vm); #endif /* __QEMU_PROCESS_H__ */ -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com
[libvirt] [PATCH 05/13] util: storage: Drop pointless 'enabled' form PR definition
Everything can be disabled by not using the parent element. There's no need to store this explicitly. Additionally it does not add any value since any configuration is dropped if enabled='no' is configured. Drop the attribute and adjust the code accordingly.t Signed-off-by: Peter Krempa--- docs/formatdomain.html.in | 21 ++-- docs/schemas/storagecommon.rng | 3 - src/util/virstoragefile.c | 117 + src/util/virstoragefile.h | 1 - .../disk-virtio-scsi-reservations.xml | 4 +- 5 files changed, 59 insertions(+), 87 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 80172c18d0..d69a669259 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2583,7 +2583,7 @@ disk type='block' device='lun' driver name='qemu' type='raw'/ source dev='/dev/sda' - reservations enabled='yes' managed='no' + reservations managed='no' source type='unix' path='/path/to/qemu-pr-helper' mode='client'/ /reservations target dev='sda' bus='scsi'/ @@ -2952,17 +2952,16 @@ Since libvirt 4.4.0, the reservations can be a sub-element of the source element for storage sources (QEMU driver only). -If present (and enabled) it enables persistent reservations for SCSI +If present it enables persistent reservations for SCSI based disks. The element has one mandatory attribute -enabled with accepted values yes and -no. If the feature is enabled, then there's another -mandatory attribute managed (accepted values are the -same as for enabled) that enables or disables libvirt -spawning a helper process. When the PR is unmanaged, then hypervisor -acts as a client and path to server socket must be provided in child -element source, which currently accepts only the -following attributes: type with one value -unix, path with path the socket, and +managed with accepted values yes and +no. If managed is enabled libvirt prepares +and manages any resources needed for the feature. When the PR is +unmanaged, then hypervisor acts as a client and path to server +socket must be provided in child element source, +which currently accepts only the following attributes: +type with one value unix, +path with path the socket, and finally mode which accepts one value client and specifies the role of hypervisor. It's recommended to allow libvirt manage the persistent diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng index eed0b33347..cb4f14f52f 100644 --- a/docs/schemas/storagecommon.rng +++ b/docs/schemas/storagecommon.rng @@ -75,9 +75,6 @@ - - - diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 87c3499561..d6907e47bb 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1906,8 +1906,8 @@ virStoragePRDefFree(virStoragePRDefPtr prd) virStoragePRDefPtr virStoragePRDefParseXML(xmlXPathContextPtr ctxt) { -virStoragePRDefPtr prd, ret = NULL; -char *enabled = NULL; +virStoragePRDefPtr prd; +virStoragePRDefPtr ret = NULL; char *managed = NULL; char *type = NULL; char *path = NULL; @@ -1916,81 +1916,65 @@ virStoragePRDefParseXML(xmlXPathContextPtr ctxt) if (VIR_ALLOC(prd) < 0) return NULL; -if (!(enabled = virXPathString("string(./@enabled)", ctxt))) { +if (!(managed = virXPathString("string(./@managed)", ctxt))) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing @enabled attribute for ")); + _("missing @managed attribute for ")); goto cleanup; } -if ((prd->enabled = virTristateBoolTypeFromString(enabled)) <= 0) { +if ((prd->managed = virTristateBoolTypeFromString(managed)) <= 0) { virReportError(VIR_ERR_XML_ERROR, - _("invalid value for 'enabled': %s"), enabled); + _("invalid value for 'managed': %s"), managed); goto cleanup; } -if (prd->enabled == VIR_TRISTATE_BOOL_YES) { -if (!(managed = virXPathString("string(./@managed)", ctxt))) { +if (prd->managed == VIR_TRISTATE_BOOL_NO) { +type = virXPathString("string(./source[1]/@type)", ctxt); +path = virXPathString("string(./source[1]/@path)", ctxt); +mode = virXPathString("string(./source[1]/@mode)", ctxt); + +if (!type) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing @managed attribute for ")); + _("missing
[libvirt] [PATCH 03/13] qemu: command: Fix comment for qemuBuildPRManagerInfoProps
The comment did not accurately describe the arguments. Signed-off-by: Peter Krempa--- src/qemu/qemu_command.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 24b482efd8..d84cf6dffc 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9667,8 +9667,10 @@ qemuBuildPanicCommandLine(virCommandPtr cmd, /** * qemuBuildPRManagerInfoProps: - * @prd: disk PR runtime info - * @propsret: JSON properties to return + * @vm: domain object + * @disk: disk definition + * @propsret: Returns JSON object containing properties of the pr-manager-helper object + * @aliasret: alias of the pr-manager-helper object * * Build the JSON properties for the pr-manager object. * -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 07/13] util: storage: Allow passing also for managed PR case
To allow storing status information in the XML move the validation that the 'path' is not valid for managed PR daemon case into qemuDomainValidateStorageSource and allow parsing of the data even in case when managed='yes'. Signed-off-by: Peter Krempa--- src/qemu/qemu_domain.c| 18 +- src/util/virstoragefile.c | 37 ++--- 2 files changed, 31 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c8d2daa26f..eaa796260c 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4204,11 +4204,19 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src, } } -if (src->pr && -!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PR_MANAGER_HELPER)) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("reservations not supported with this QEMU binary")); -return -1; +if (src->pr) { +if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PR_MANAGER_HELPER)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("reservations not supported with this QEMU binary")); +return -1; +} + +if (virStoragePRDefIsManaged(src->pr) && src->pr->path) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'path' attribute should not be provided for " + "managed reservations")); +return -1; +} } return 0; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index c89bdb9e49..dbbe758f30 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1928,11 +1928,11 @@ virStoragePRDefParseXML(xmlXPathContextPtr ctxt) goto cleanup; } -if (prd->managed == VIR_TRISTATE_BOOL_NO) { -type = virXPathString("string(./source[1]/@type)", ctxt); -path = virXPathString("string(./source[1]/@path)", ctxt); -mode = virXPathString("string(./source[1]/@mode)", ctxt); +type = virXPathString("string(./source[1]/@type)", ctxt); +path = virXPathString("string(./source[1]/@path)", ctxt); +mode = virXPathString("string(./source[1]/@mode)", ctxt); +if (prd->managed == VIR_TRISTATE_BOOL_NO || type || path || mode) { if (!type) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing connection type for ")); @@ -1950,24 +1950,23 @@ virStoragePRDefParseXML(xmlXPathContextPtr ctxt) _("missing connection mode for ")); goto cleanup; } +} -if (STRNEQ(type, "unix")) { -virReportError(VIR_ERR_XML_ERROR, - _("unsupported connection type for : %s"), - type); -goto cleanup; -} - -if (STRNEQ(mode, "client")) { -virReportError(VIR_ERR_XML_ERROR, - _("unsupported connection mode for : %s"), - mode); -goto cleanup; -} +if (type && STRNEQ(type, "unix")) { +virReportError(VIR_ERR_XML_ERROR, + _("unsupported connection type for : %s"), + type); +goto cleanup; +} -VIR_STEAL_PTR(prd->path, path); +if (mode && STRNEQ(mode, "client")) { +virReportError(VIR_ERR_XML_ERROR, + _("unsupported connection mode for : %s"), + mode); +goto cleanup; } +VIR_STEAL_PTR(prd->path, path); VIR_STEAL_PTR(ret, prd); cleanup: @@ -1986,7 +1985,7 @@ virStoragePRDefFormat(virBufferPtr buf, { virBufferAsprintf(buf, "managed)); -if (prd->managed == VIR_TRISTATE_BOOL_NO) { +if (prd->path) { virBufferAddLit(buf, ">\n"); virBufferAdjustIndent(buf, 2); virBufferAddLit(buf, "https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 02/13] qemu: alias: Allow passing alias of parent when generating PR manager alias
For use with blockdev the PR manager will be bound to a virStorageSource rather than a virDomainDiskDef, so we will need to use the correct alias. Allow passing a string rather than the whole disk. Signed-off-by: Peter Krempa--- src/qemu/qemu_alias.c | 4 ++-- src/qemu/qemu_alias.h | 2 +- src/qemu/qemu_command.c | 4 ++-- src/qemu/qemu_hotplug.c | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index bd714f7aee..578a33b284 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -783,11 +783,11 @@ qemuDomainGetManagedPRAlias(void) char * -qemuDomainGetUnmanagedPRAlias(const virDomainDiskDef *disk) +qemuDomainGetUnmanagedPRAlias(const char *parentalias) { char *ret; -ignore_value(virAsprintf(, "pr-helper-%s", disk->info.alias)); +ignore_value(virAsprintf(, "pr-helper-%s", parentalias)); return ret; } diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h index 76678658c0..51f64624d7 100644 --- a/src/qemu/qemu_alias.h +++ b/src/qemu/qemu_alias.h @@ -94,6 +94,6 @@ char *qemuAliasChardevFromDevAlias(const char *devAlias) const char *qemuDomainGetManagedPRAlias(void); -char *qemuDomainGetUnmanagedPRAlias(const virDomainDiskDef *disk); +char *qemuDomainGetUnmanagedPRAlias(const char *parentalias); #endif /* __QEMU_ALIAS_H__*/ diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 11ad77f145..24b482efd8 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1482,7 +1482,7 @@ qemuBuildDriveSourcePR(virBufferPtr buf, if (virStoragePRDefIsManaged(disk->src->pr)) defaultAlias = qemuDomainGetManagedPRAlias(); -else if (!(alias = qemuDomainGetUnmanagedPRAlias(disk))) +else if (!(alias = qemuDomainGetUnmanagedPRAlias(disk->info.alias))) return -1; @@ -9705,7 +9705,7 @@ qemuBuildPRManagerInfoProps(virDomainObjPtr vm, if (VIR_STRDUP(alias, qemuDomainGetManagedPRAlias()) < 0) goto cleanup; } else { -if (!(alias = qemuDomainGetUnmanagedPRAlias(disk))) +if (!(alias = qemuDomainGetUnmanagedPRAlias(disk->info.alias))) goto cleanup; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1d748ccffb..52e1abdcd3 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3842,7 +3842,7 @@ qemuDomainDiskNeedRemovePR(virDomainObjPtr vm, return 0; if (!virStoragePRDefIsManaged(disk->src->pr)) { -*aliasret = qemuDomainGetUnmanagedPRAlias(disk); +*aliasret = qemuDomainGetUnmanagedPRAlias(disk->info.alias); return *aliasret ? 0 : -1; } -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 06/13] util: storage: Drop virStoragePRDefIsEnabled
The function now does not do anything useful. Replace it by the pointer check. Signed-off-by: Peter Krempa--- src/libvirt_private.syms | 1 - src/qemu/qemu_command.c | 4 ++-- src/qemu/qemu_domain.c| 8 src/qemu/qemu_hotplug.c | 2 +- src/util/virstoragefile.c | 7 --- src/util/virstoragefile.h | 1 - 6 files changed, 7 insertions(+), 16 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d28a751ebd..dd10be9753 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2804,7 +2804,6 @@ virStorageNetHostTransportTypeToString; virStorageNetProtocolTypeToString; virStoragePRDefFormat; virStoragePRDefFree; -virStoragePRDefIsEnabled; virStoragePRDefIsEqual; virStoragePRDefIsManaged; virStoragePRDefParseXML; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 29ca2005a0..2bdba7734a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1477,7 +1477,7 @@ qemuBuildDriveSourcePR(virBufferPtr buf, char *alias = NULL; const char *defaultAlias = NULL; -if (!virStoragePRDefIsEnabled(disk->src->pr)) +if (!disk->src->pr) return 0; if (virStoragePRDefIsManaged(disk->src->pr)) @@ -9690,7 +9690,7 @@ qemuBuildPRManagerInfoProps(virDomainObjPtr vm, *propsret = NULL; *aliasret = NULL; -if (!virStoragePRDefIsEnabled(disk->src->pr)) +if (!disk->src->pr) return 0; if (!(socketPath = qemuDomainGetPRSocketPath(vm, disk->src->pr))) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 611a96d6be..c8d2daa26f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4204,7 +4204,7 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src, } } -if (virStoragePRDefIsEnabled(src->pr) && +if (src->pr && !virQEMUCapsGet(qemuCaps, QEMU_CAPS_PR_MANAGER_HELPER)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("reservations not supported with this QEMU binary")); @@ -10240,7 +10240,7 @@ qemuDomainSetupDisk(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED, } /* qemu-pr-helper might require access to /dev/mapper/control. */ -if (virStoragePRDefIsEnabled(disk->src->pr) && +if (disk->src->pr && qemuDomainCreateDevice(DEVICE_MAPPER_CONTROL_PATH, data, true) < 0) goto cleanup; @@ -11273,7 +11273,7 @@ qemuDomainNamespaceSetupDisk(virDomainObjPtr vm, } /* qemu-pr-helper might require access to /dev/mapper/control. */ -if (virStoragePRDefIsEnabled(src->pr) && +if (src->pr && (VIR_STRDUP(dmPath, DEVICE_MAPPER_CONTROL_PATH) < 0 || VIR_APPEND_ELEMENT_COPY(paths, npaths, dmPath) < 0)) goto cleanup; @@ -12050,7 +12050,7 @@ qemuDomainGetPRSocketPath(virDomainObjPtr vm, const char *defaultAlias = NULL; char *ret = NULL; -if (!virStoragePRDefIsEnabled(pr)) +if (!pr) return NULL; if (virStoragePRDefIsManaged(pr)) { diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 52e1abdcd3..39c457e607 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3838,7 +3838,7 @@ qemuDomainDiskNeedRemovePR(virDomainObjPtr vm, *aliasret = NULL; *stopDaemon = false; -if (!virStoragePRDefIsEnabled(disk->src->pr)) +if (!disk->src->pr) return 0; if (!virStoragePRDefIsManaged(disk->src->pr)) { diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index d6907e47bb..c89bdb9e49 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2018,13 +2018,6 @@ virStoragePRDefIsEqual(virStoragePRDefPtr a, } -bool -virStoragePRDefIsEnabled(virStoragePRDefPtr prd) -{ -return !!prd; -} - - bool virStoragePRDefIsManaged(virStoragePRDefPtr prd) { diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index ec49152880..3a90c60fa5 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -396,7 +396,6 @@ void virStoragePRDefFormat(virBufferPtr buf, virStoragePRDefPtr prd); bool virStoragePRDefIsEqual(virStoragePRDefPtr a, virStoragePRDefPtr b); -bool virStoragePRDefIsEnabled(virStoragePRDefPtr prd); bool virStoragePRDefIsManaged(virStoragePRDefPtr prd); virSecurityDeviceLabelDefPtr -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/13] qemu: Assign managed PR path when preparing storage source
Rather than always checking which path to use pre-assign it when preparing storage source. This reduces the need to pass 'vm' around too much. For later use the path can be retrieved from the status XML. Signed-off-by: Peter Krempa--- src/qemu/qemu_command.c | 18 +- src/qemu/qemu_command.h | 3 +-- src/qemu/qemu_domain.c | 35 ++- src/qemu/qemu_domain.h | 3 +-- src/qemu/qemu_hotplug.c | 2 +- src/qemu/qemu_process.c | 2 +- 6 files changed, 31 insertions(+), 32 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 2bdba7734a..7df9979cb2 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9667,7 +9667,6 @@ qemuBuildPanicCommandLine(virCommandPtr cmd, /** * qemuBuildPRManagerInfoProps: - * @vm: domain object * @disk: disk definition * @propsret: Returns JSON object containing properties of the pr-manager-helper object * @aliasret: alias of the pr-manager-helper object @@ -9678,12 +9677,10 @@ qemuBuildPanicCommandLine(virCommandPtr cmd, * -1 on failure (with error message set). */ int -qemuBuildPRManagerInfoProps(virDomainObjPtr vm, -const virDomainDiskDef *disk, +qemuBuildPRManagerInfoProps(const virDomainDiskDef *disk, virJSONValuePtr *propsret, char **aliasret) { -char *socketPath = NULL; char *alias = NULL; int ret = -1; @@ -9693,9 +9690,6 @@ qemuBuildPRManagerInfoProps(virDomainObjPtr vm, if (!disk->src->pr) return 0; -if (!(socketPath = qemuDomainGetPRSocketPath(vm, disk->src->pr))) -return ret; - if (virStoragePRDefIsManaged(disk->src->pr)) { if (VIR_STRDUP(alias, qemuDomainGetManagedPRAlias()) < 0) goto cleanup; @@ -9705,7 +9699,7 @@ qemuBuildPRManagerInfoProps(virDomainObjPtr vm, } if (virJSONValueObjectCreate(propsret, - "s:path", socketPath, + "s:path", disk->src->pr->path, NULL) < 0) goto cleanup; @@ -9713,14 +9707,12 @@ qemuBuildPRManagerInfoProps(virDomainObjPtr vm, ret = 0; cleanup: VIR_FREE(alias); -VIR_FREE(socketPath); return ret; } static int -qemuBuildMasterPRCommandLine(virDomainObjPtr vm, - virCommandPtr cmd, +qemuBuildMasterPRCommandLine(virCommandPtr cmd, const virDomainDef *def) { size_t i; @@ -9740,7 +9732,7 @@ qemuBuildMasterPRCommandLine(virDomainObjPtr vm, managedAdded = true; } -if (qemuBuildPRManagerInfoProps(vm, disk, , ) < 0) +if (qemuBuildPRManagerInfoProps(disk, , ) < 0) goto cleanup; if (!props) @@ -9934,7 +9926,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildMasterKeyCommandLine(cmd, priv) < 0) goto error; -if (qemuBuildMasterPRCommandLine(vm, cmd, def) < 0) +if (qemuBuildMasterPRCommandLine(cmd, def) < 0) goto error; if (enableFips) diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index da1fe679fe..621592cd79 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -55,8 +55,7 @@ virCommandPtr qemuBuildCommandLine(virQEMUDriverPtr driver, int **nicindexes); /* Generate the object properties for pr-manager */ -int qemuBuildPRManagerInfoProps(virDomainObjPtr vm, -const virDomainDiskDef *disk, +int qemuBuildPRManagerInfoProps(const virDomainDiskDef *disk, virJSONValuePtr *propsret, char **alias); diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index eaa796260c..92217e66fe 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11930,6 +11930,22 @@ qemuDomainPrepareDiskCachemode(virDomainDiskDefPtr disk) } +static int +qemuDomainPrepareStorageSourcePR(virStorageSourcePtr src, + qemuDomainObjPrivatePtr priv) +{ +if (!src->pr) +return 0; + +if (virStoragePRDefIsManaged(src->pr)) { +if (!(src->pr->path = qemuDomainGetManagedPRSocketPath(priv))) +return -1; +} + +return 0; +} + + int qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk, qemuDomainObjPrivatePtr priv, @@ -11946,6 +11962,9 @@ qemuDomainPrepareDiskSource(virDomainDiskDefPtr disk, if (qemuDomainPrepareDiskSourceChain(disk, NULL, cfg, priv->qemuCaps) < 0) return -1; +if (qemuDomainPrepareStorageSourcePR(disk->src, priv) < 0) +return -1; + return 0; } @@ -12051,22 +12070,12 @@ qemuProcessEventFree(struct qemuProcessEvent *event) char * -qemuDomainGetPRSocketPath(virDomainObjPtr vm, - virStoragePRDefPtr pr)
[libvirt] [PATCH 04/13] qemu: Move validation of PR manager support
Disk source definition should be validated in qemuDomainValidateStorageSource rather than in individual generators of command line arguments. Change to the XML2XML test is required since now the definition is actually validated at define time. Signed-off-by: Peter Krempa--- src/qemu/qemu_command.c | 7 --- src/qemu/qemu_domain.c | 7 +++ tests/qemuxml2xmltest.c | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index d84cf6dffc..29ca2005a0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9683,7 +9683,6 @@ qemuBuildPRManagerInfoProps(virDomainObjPtr vm, virJSONValuePtr *propsret, char **aliasret) { -qemuDomainObjPrivatePtr priv = vm->privateData; char *socketPath = NULL; char *alias = NULL; int ret = -1; @@ -9694,12 +9693,6 @@ qemuBuildPRManagerInfoProps(virDomainObjPtr vm, if (!virStoragePRDefIsEnabled(disk->src->pr)) return 0; -if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_PR_MANAGER_HELPER)) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("reservations not supported with this QEMU binary")); -return ret; -} - if (!(socketPath = qemuDomainGetPRSocketPath(vm, disk->src->pr))) return ret; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 233327b906..611a96d6be 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -4204,6 +4204,13 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src, } } +if (virStoragePRDefIsEnabled(src->pr) && +!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PR_MANAGER_HELPER)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("reservations not supported with this QEMU binary")); +return -1; +} + return 0; } diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 762e0e2d25..44cba97d4a 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -388,7 +388,7 @@ mymain(void) DO_TEST("disk-virtio-scsi-num_queues", QEMU_CAPS_VIRTIO_SCSI); DO_TEST("disk-virtio-scsi-reservations", -QEMU_CAPS_VIRTIO_SCSI); +QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_PR_MANAGER_HELPER); DO_TEST("disk-virtio-scsi-cmd_per_lun", QEMU_CAPS_VIRTIO_SCSI); DO_TEST("disk-virtio-scsi-max_sectors", -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 00/13] Fix XML for persistent reservations and refactor
The XML design for the PR stuff is slightly weird so fix it and refactor the code so that it will be much easier to use it with the blockdev infrastructure. Peter Krempa (13): qemu: hotplug: Fix spacing around addition operator qemu: alias: Allow passing alias of parent when generating PR manager alias qemu: command: Fix comment for qemuBuildPRManagerInfoProps qemu: Move validation of PR manager support util: storage: Drop pointless 'enabled' form PR definition util: storage: Drop virStoragePRDefIsEnabled util: storage: Allow passing also for managed PR case qemu: Assign managed PR path when preparing storage source qemu: process: Change semantics of functions starting PR daemon qemu: command: Move check whether PR manager object props need to be built conf: domain: Add helper to check whether a domain def requires use of PR util: storage: Store PR manager alias in the definition qemu: hotplug: Replace qemuDomainDiskNeedRemovePR docs/formatdomain.html.in | 21 ++-- docs/schemas/storagecommon.rng | 3 - src/conf/domain_conf.c | 21 src/conf/domain_conf.h | 3 + src/libvirt_private.syms | 2 +- src/qemu/qemu_alias.c | 4 +- src/qemu/qemu_alias.h | 2 +- src/qemu/qemu_command.c| 61 +++--- src/qemu/qemu_command.h| 6 +- src/qemu/qemu_domain.c | 66 --- src/qemu/qemu_domain.h | 3 +- src/qemu/qemu_hotplug.c| 83 +++--- src/qemu/qemu_process.c| 40 ++- src/qemu/qemu_process.h| 5 +- src/util/virstoragefile.c | 124 - src/util/virstoragefile.h | 5 +- tests/qemustatusxml2xmldata/modern-in.xml | 4 + .../disk-virtio-scsi-reservations.xml | 4 +- tests/qemuxml2xmltest.c| 2 +- 19 files changed, 191 insertions(+), 268 deletions(-) -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 01/13] qemu: hotplug: Fix spacing around addition operator
Signed-off-by: Peter Krempa--- src/qemu/qemu_hotplug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index f3ff945787..1d748ccffb 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -484,7 +484,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps))) goto error; -if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks+1) < 0) +if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks + 1) < 0) goto error; qemuDomainObjEnterMonitor(driver, vm); -- 2.16.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH v2 3/3] virtDBusNetworkGetDHCPLeases: fix type for expirytime
On Fri, May 11, 2018 at 06:16:56PM +0200, Katerina Koukiou wrote: > Signed-off-by: Katerina Koukiou> --- > data/org.libvirt.Network.xml | 2 +- > src/network.c| 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH v2 2/3] Replace uint -> int wherever libvirt uses int type
On Fri, May 11, 2018 at 06:16:55PM +0200, Katerina Koukiou wrote: > Follow this pattern even if negative values will not > appear, in order to be consistent with libvirt APIs. > > Signed-off-by: Katerina Koukiou> --- > Added type change for MemoryStats > > data/org.libvirt.Connect.xml | 18 +- > data/org.libvirt.Domain.xml | 18 +- > data/org.libvirt.Network.xml | 2 +- > data/org.libvirt.Secret.xml | 2 +- > data/org.libvirt.StoragePool.xml | 2 +- > src/connect.c| 6 +++--- > src/domain.c | 22 +++--- > src/events.c | 12 ++-- > src/network.c| 4 ++-- > src/secret.c | 2 +- > src/storagepool.c| 2 +- > 11 files changed, 45 insertions(+), 45 deletions(-) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH v2 1/3] virtDBusNetworkGetDHCPLeases: fix lease type
On Fri, May 11, 2018 at 06:16:54PM +0200, Katerina Koukiou wrote: > Signed-off-by: Katerina Koukiou> --- > src/network.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Some explanation would be nice, something like: Commit 8eac355f3e removed *TypedToString conversion but missed to update this one place. Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH v2 3/3] events: Register VIR_DOMAIN_EVENT_ID_IO_ERROR_REASON
On Fri, May 11, 2018 at 05:49:23PM +0200, Katerina Koukiou wrote: > Signed-off-by: Katerina Koukiou> --- > Return empty string when variable is NULL > > data/org.libvirt.Domain.xml | 8 > src/events.c| 31 +++ > 2 files changed, 39 insertions(+) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH v2 2/3] events: Register VIR_DOMAIN_EVENT_ID_GRAPHICS
On Fri, May 11, 2018 at 05:49:22PM +0200, Katerina Koukiou wrote: > Signed-off-by: Katerina Koukiou> --- > Return empty string when variable is NULL > > data/org.libvirt.Domain.xml | 9 +++ > src/events.c| 60 > + > src/util.h | 2 ++ > 3 files changed, 71 insertions(+) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH v2 1/3] events: Register VIR_DOMAIN_EVENT_ID_BLOCK_JOB_2
On Fri, May 11, 2018 at 05:49:21PM +0200, Katerina Koukiou wrote: > Signed-off-by: Katerina Koukiou> --- > Use only v2 of BlockJob > > data/org.libvirt.Domain.xml | 8 > src/events.c| 28 > 2 files changed, 36 insertions(+) Reviewed-by: Pavel Hrdina signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] travis: Uninstall packages before upgrade
On Mon, May 14, 2018 at 12:07:45PM +0200, Andrea Bolognani wrote: > numpy (needed by cgal) started having the same issue with > linking as python, which makes upgrade and thus the entire > build fail on macOS. > > Instead of playing more tricks with linking/unlinking, just > uninstall the problematic packages (and those dragging them > in) before doing anything else. > > Signed-off-by: Andrea Bolognani> --- > Technically a build breaker, but since I'm changing the > approach I'd rather wait for an explicit ACK before pushing. > > .travis.yml | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/.travis.yml b/.travis.yml > index d3f72d46f3..140072b801 100644 > --- a/.travis.yml > +++ b/.travis.yml > @@ -21,10 +21,10 @@ matrix: > - compiler: clang >os: osx >before_install: > +- brew uninstall python mercurial postgis sfcgal cgal gdal > - brew update > -- brew unlink python > - brew upgrade > -- brew install rpcgen yajl > +- brew install python rpcgen yajl >script: > # We can't run make distcheck/syntax-check because they > # fail on macOS, but doing 'install' and 'dist' gives us 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
[libvirt] [PATCH] travis: Uninstall packages before upgrade
numpy (needed by cgal) started having the same issue with linking as python, which makes upgrade and thus the entire build fail on macOS. Instead of playing more tricks with linking/unlinking, just uninstall the problematic packages (and those dragging them in) before doing anything else. Signed-off-by: Andrea Bolognani--- Technically a build breaker, but since I'm changing the approach I'd rather wait for an explicit ACK before pushing. .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index d3f72d46f3..140072b801 100644 --- a/.travis.yml +++ b/.travis.yml @@ -21,10 +21,10 @@ matrix: - compiler: clang os: osx before_install: +- brew uninstall python mercurial postgis sfcgal cgal gdal - brew update -- brew unlink python - brew upgrade -- brew install rpcgen yajl +- brew install python rpcgen yajl script: # We can't run make distcheck/syntax-check because they # fail on macOS, but doing 'install' and 'dist' gives us -- 2.17.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] Introduce virCryptoHashBuf
On 05/11/2018 05:50 PM, Ján Tomko wrote: > A function that keeps the hash in binary form instead of converting > it to human-readable hexadecimal form. > > Signed-off-by: Ján Tomko> --- > src/util/vircrypto.c | 31 +-- > src/util/vircrypto.h | 7 +++ > 2 files changed, 28 insertions(+), 10 deletions(-) > > diff --git a/src/util/vircrypto.c b/src/util/vircrypto.c > index 48b04fc8ce..1a2dcc28b7 100644 > --- a/src/util/vircrypto.c > +++ b/src/util/vircrypto.c > @@ -54,28 +54,39 @@ struct virHashInfo { > verify(ARRAY_CARDINALITY(hashinfo) == VIR_CRYPTO_HASH_LAST); > > int > -virCryptoHashString(virCryptoHash hash, > -const char *input, > -char **output) > +virCryptoHashBuf(virCryptoHash hash, > + const char *input, > + unsigned char *output) > { > -unsigned char buf[VIR_CRYPTO_LARGEST_DIGEST_SIZE]; > -size_t hashstrlen; > -size_t i; > - > if (hash >= VIR_CRYPTO_HASH_LAST) { > virReportError(VIR_ERR_INVALID_ARG, > _("Unknown crypto hash %d"), hash); > return -1; > } This check feels useless. It's like if we were checking a value before passing it to vir*TypeToString(). But it's pre-existing, so you have my ACK. But a follow up patch removing it (=trivial) would be nice. Also, don't forget to export the symbol in libvirt_private.syms ;-) I'm wondering how linker succeeds in 3/5 where you use the function without it being exported. Maybe my compiler decided to inline this function? Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] Rely on GnuTLS for md5/sha256 functions
On 05/11/2018 05:50 PM, Ján Tomko wrote: > Ján Tomko (5): > vircrypto: provide constants for hash sizes > Introduce virCryptoHashBuf > esx: use virCryptoHashBuf > esx: Use VIR_CRYPTO_HASH_SIZE_MD5 > vircrypto: Rely on GnuTLS for hash functions > > bootstrap.conf | 2 -- > src/esx/esx_network_driver.c| 22 -- > src/esx/esx_storage_backend_iscsi.c | 46 +- > src/esx/esx_storage_backend_vmfs.c | 20 ++--- > src/util/vircrypto.c| 57 > ++--- > src/util/vircrypto.h| 10 +++ > 6 files changed, 100 insertions(+), 57 deletions(-) > ACK series, but please see my comment in 2/5 before pushing. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/8] qemu: vfio-ccw device address generation
On 05/10/2018 10:52 PM, John Ferlan wrote: On 05/07/2018 10:41 AM, Boris Fiuczynski wrote: From: Shalini Chellathurai SarojaIntroduces the vfio-ccw model for mediated devices and prime vfio-ccw devices such that CCW address will be generated. Signed-off-by: Shalini Chellathurai Saroja Reviewed-by: Bjoern Walk Reviewed-by: Boris Fiuczynski Reviewed-by: Marc Hartmayer Reviewed-by: Stefan Zimmermann --- docs/schemas/domaincommon.rng | 5 - src/qemu/qemu_domain_address.c | 20 src/util/virmdev.c | 3 ++- src/util/virmdev.h | 1 + 4 files changed, 27 insertions(+), 2 deletions(-) Looking at all places using VIR_MDEV_MODEL_TYPE_VFIO_PCI - should this patch make a change to virDomainHostdevDefPostParse to do something similar - that is: if (model == VIR_MDEV_MODEL_TYPE_VFIO_CCW && dev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { ... error message } ? Let me know... I think it should and can add it before pushing... You are correct. Good catch! How about fixing it like this? if ((model == VIR_MDEV_MODEL_TYPE_VFIO_PCI && dev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) || (model == VIR_MDEV_MODEL_TYPE_VFIO_CCW && dev->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW)) { virReportError(VIR_ERR_XML_ERROR, _("Unsupported address type '%s' with mediated " "device model '%s'"), virDomainDeviceAddressTypeToString(dev->info->type), virMediatedDeviceModelTypeToString(model)); return -1; } Besides that I just saw that the indention of the second parameter of method qemuDomainPrimeVfioDeviceAddresses is off by three blanks. Reviewed-by: John Ferlan John -- 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: Martina Köderitz 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] [dbus PATCH] connect: Remove unused variable
On Mon, May 14, 2018 at 09:13:19AM +0200, Andrea Bolognani wrote: Clang catches the issue: ../../src/connect.c:532:26: error: unused variable 'domain' [-Werror,-Wunused-variable] g_autoptr(virDomain) domain = NULL; ^ Signed-off-by: Andrea Bolognani--- src/connect.c | 1 - 1 file changed, 1 deletion(-) Reviewed-by: Ján Tomko Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dbus PATCH] connect: Remove unused variable
Clang catches the issue: ../../src/connect.c:532:26: error: unused variable 'domain' [-Werror,-Wunused-variable] g_autoptr(virDomain) domain = NULL; ^ Signed-off-by: Andrea Bolognani--- src/connect.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/connect.c b/src/connect.c index 6719e21..14a60dc 100644 --- a/src/connect.c +++ b/src/connect.c @@ -529,7 +529,6 @@ virtDBusConnectDomainSaveImageGetXMLDesc(GVariant *inArgs, GError **error) { virtDBusConnect *connect = userData; -g_autoptr(virDomain) domain = NULL; const gchar *file; guint flags; g_autofree gchar *xml = NULL; -- 2.17.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] Finish replacements for virDomainObjCheckActive
On Mon, May 14, 2018 at 12:32:12AM +0200, c...@lse.epita.fr wrote: From: Clementine HayatFollow-up of https://www.redhat.com/archives/libvir-list/2018-April/msg01591.html Here is the changes asked by Ján Tomko: * Refactor also checks when the error message is close to "domain is not running" * Add forgotten checks in qemu_migration.c * Revert one incorrect change in lxc_driver.c Clementine Hayat (3): qemu: start using virDomainObjCheckActive lxc: start using virDomainObjCheckActive bhyve: start using virDomainObjCheckActive src/bhyve/bhyve_driver.c | 15 +-- src/lxc/lxc_driver.c | 65 ++--- src/qemu/qemu_domain.c| 5 +- src/qemu/qemu_driver.c| 271 -- src/qemu/qemu_migration.c | 10 +- 5 files changed, 74 insertions(+), 292 deletions(-) Reviewed-by: Ján Tomko And pushed. Jano signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] bhyve: allow locking memory
Peter Krempa wrote: > On Sun, May 13, 2018 at 14:01:25 +0400, Roman Bogorodskiy wrote: > > Fabian Freyer wrote: > > [...] > > > > 13 files changed, 134 insertions(+) > > > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-wired.args > > > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-wired.xml > > > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-wired.args > > > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-wired.ldargs > > > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-wired.xml > > > create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-wired.xml > > > > Thanks for the patches, I've pushed the series with some syntax-check > > fixes (trailing whitespaces, tabs instead of whitespaces) and added > > 'Signed-off-by' line which is mandatory now. > > Note that 'Signed-off-by' should _NOT_ be added without explicit consent > of the author of the patches. Otherwise it defeats the purpose of > certification that the author agrees and complies with the Developer > certificate of origin. Adding it without explicit consent makes the > Signed-off-by line just a useless piece of jewelery added to appease the > push hook. Oh, sorry about that. I assumed as Fabian is a long time contributor, he's familiar with the general project policies. Fabian, do you have any issue with that? > > > > Roman Bogorodskiy > > > > > -- > > libvir-list mailing list > > libvir-list@redhat.com > > https://www.redhat.com/mailman/listinfo/libvir-list > Roman Bogorodskiy signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/8] qemu: introduce capability for virtual-css-bridge
On 05/10/2018 10:46 PM, John Ferlan wrote: On 05/07/2018 10:41 AM, Boris Fiuczynski wrote: From: Shalini Chellathurai SarojaLet us introduce the capability QEMU_CAPS_CCW for virtual-css-bridge and replace QEMU_CAPS_VIRTIO_CCW with QEMU_CAPS_CCW in code segments which identify support for ccw devices. The virtual-css-bridge is part of the ccw support introduced in QEMU 2.7. The QEMU_CAPS_CCW capability is based on the existence of the QEMU type. Let us also add the capability QEMU_CAPS_CCW to the tests which require support for ccw devices. Signed-off-by: Shalini Chellathurai Saroja Reviewed-by: Bjoern Walk Reviewed-by: Boris Fiuczynski --- src/qemu/qemu_capabilities.c | 9 +++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_domain_address.c | 4 +- src/qemu/qemu_hotplug.c | 4 +- tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + tests/qemuhotplugtest.c | 2 +- tests/qemuxml2argvtest.c | 86 tests/qemuxml2xmltest.c | 26 +++ 13 files changed, 77 insertions(+), 62 deletions(-) tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml was missed on this patch - make check will fail, so it needs to be adjusted now so that bisection doesn't some day have problems. I see it was added in patch 2 and that IOW: + You are right of course. It is a miss on my part. Patch 2 contains the change and I was to lazy to run an iterated apply patch and build which would have caught this. Sorry. You may also want to send a "final" v2.12.0 replies/caps patch later too... You are right and Shalini has already prepared a patch to do so. She will send it out soon. Reviewed-by: John Ferlan John Thanks -- 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: Martina Köderitz 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 0/3] bhyve: allow locking memory
On Sun, May 13, 2018 at 14:01:25 +0400, Roman Bogorodskiy wrote: > Fabian Freyer wrote: [...] > > 13 files changed, 134 insertions(+) > > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-wired.args > > create mode 100644 tests/bhyveargv2xmldata/bhyveargv2xml-wired.xml > > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-wired.args > > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-wired.ldargs > > create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-wired.xml > > create mode 100644 tests/bhyvexml2xmloutdata/bhyvexml2xmlout-wired.xml > > Thanks for the patches, I've pushed the series with some syntax-check > fixes (trailing whitespaces, tabs instead of whitespaces) and added > 'Signed-off-by' line which is mandatory now. Note that 'Signed-off-by' should _NOT_ be added without explicit consent of the author of the patches. Otherwise it defeats the purpose of certification that the author agrees and complies with the Developer certificate of origin. Adding it without explicit consent makes the Signed-off-by line just a useless piece of jewelery added to appease the push hook. > > Roman Bogorodskiy > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list