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 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
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
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
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, &props, &alias) < 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
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 Krempa
Something 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
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 @@
>
>
>
> -
> +
> mode='client'/>
>
>
> @@ -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
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
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 Freyer
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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
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
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
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
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
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
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
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
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
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
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
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
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
$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
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
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
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
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 @@
...
+Where loader/NVRAM can also be described as:
+
+
+...
+
+
+
+
+
+
+
+
+...
+
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 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
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 @@
-
+
@@ -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 connection
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
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
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
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
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;
}
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
On Mon, May 14, 2018 at 12:32:12AM +0200, c...@lse.epita.fr wrote:
From: Clementine Hayat
Follow-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
