Re: [libvirt] [PATCH] qemu: use 'bochs' video type by default for UEFI domains
On Thu, Aug 29, 2019 at 12:21 PM Gerd Hoffmann wrote: > > Hi, > > > Incidentally, and I haven't followed this closely so apologies if I'm > > asking a silly question, if the point of bochs-display is to have a > > video device without legacy VGA emulation then why aren't we using > > the existing virtio-gpu, which has been around a lot longer and has > > better support throughout the stack, instead? > > Well, virtio-gpu (on x86) actually is "-device virtio-vga". That comes > with legacy VGA emulation, for the boot display ... > > For UEFI guests with virtio-gpu support it is possible to use "-display > virtio-gpu-pci" instead (simliar to arm). Only drawback is that you > can't have a EFI GOB with virtio-gpu-pci, so efifb doesn't work. Linux > kernel console shows up after the virtio-gpu driver loads, which is > rather late compared to efifb. > > For UEFI guests without virtio-gpu support "-device virtio-gpu-pci" will > not work due to the lack of EFI GOB support. For these guests > "-display bochs-display" should preferred over "-device VGA", to get rid > of the unused legacy VGA emulation (and thereby reduce the attack > surface). > > Also note that bochs-display can be plugged into pcie slots (that is > true for virtio-gpu-pci too btw). > > Not sure whenever implementing this works better in libvirt or > libosinfo. I do believe this piece should go to libosinfo. However, the decision of what exactly to use due to lack of this or that device, should be done in the management apps, in a similar way of what was done for the q35 work. > > cheers, > Gerd > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [BUG] When PLUG a bridge interface to an active VM, the generated LIVE and CONFIG mac address are different
On 8/30/19 8:51 AM, wujing (O) wrote:
On 8/28/19 11:42 AM, wujing (O) wrote:
On 8/22/19 10:43 AM, Xu Yandong (Yandong Xu) wrote:
That possibly might be out of scope, but autofilling the mac
address as early as virDomainNetDefParseXML also is not ideal.
We have pushed a patch bellow that can restore the situation to an
older
state.
Subject: [PATCH] qemu: use the same def when attaching device live
and config
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index
617d7d5..eca54d0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8615,6 +8615,22 @@
qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
goto cleanup;
+if (flags & VIR_DOMAIN_AFFECT_CONFIG &&
+flags & VIR_DOMAIN_AFFECT_LIVE) {
+/* If we are affecting both CONFIG and LIVE
+ * use the same xml of device preferentially
+ * to make the configuration consistent.
+ */
+devLive = virDomainDeviceDefParse(xml, vm->def,
This use of vm->def prefers live XML. This means that for instance
PCI address are assigned based on current PCI layout in live XML
which in general is different to inactive XML.
And it's not only PCI addresses, we autogenerate some out aspects of
(e.g. model) and even more for other devices. Just take
a look at qemuDomainDeviceDefPostParse(). All functions there which
take domain def as an argument do so because they are basing
autogenerated value on domain definition. Plus there's more in parser
code.
While your patch might work for your use case, it can break others.
BTW: have you read the original commit that caused this? I'm failing
to see how we would not re-introduce the problem with your patch. If
you're using live XML to validate/generate device addresses, how can
we generate valid address for inactive XML?
This patch is a little different with the original commit, since we
only prefer live XML when using LIVE AND CONFIG flags at the same time.
Still parse separate device def if attach with only one flag.
So this patch is kinda a combination of 1e0534a7702 and 55ce6564634, just
to narrow down the problem influence.
And I verified as the steps in
https://bugzilla.redhat.com/show_bug.cgi?id=1559867, work as expected.
Well, then try to coldplug (that is
only to inactive XML), and then try hotplug different to
both live and inactive XMLs and the bug will reproduce.
Problem is, that inactive XML contains a device which is not contained in live
XML and thus when generating device address based solely on live XML a
conflicting address is generated.
If we'd prefer inactive XML in your patch then the steps in my reproducer
need to be swapped. At any rate, the issue is still there.
You are right, but I found a new issue on current solution.
Test as steps below.
1. Having a running vm
## virsh list
IdName State
-- │~
1rhel running
2. prepare 2 scsi hostdev xml using the same address
## cat h1.xml
## cat h2.xml
3. attach 2 hostdev to vm with 'attach-device --config'
## virsh attach-device rhel h1.xml --config
Device attached successfully
## virsh attach-device rhel h2.xml --config
Device attached successfully
4. Check the vm's xml with 'dumpxml --inactive'
## virsh dumpxml rhel --inactive
...
...
a) Looks like libvirt can not verify if user configured the same address for
different scsi hostdevs.
Because virDomainHostdevDefPostParse only calls
virDomainDriveAddressIsUsedByDisk to check
if address is used by other disks, but no check if used by other hostdevs.
And qemuDomainAttachDeviceConfig calls virDomainHostdevFind to check duplicate
devices by
src addr and other stuff, not including addr in vm.
Should that be fixed?
Oh yeah, this is a long known issue:
https://bugzilla.redhat.com/show_bug.cgi?id=1729022
and I want to fix it, but I always get side tracked :-( So if somebody
beats me to it, I wouldn't mind ;-)
b) If we fixed a), then we can get a devConf that both fits to live and config
xml by
passing devLive and domainPersistentDef to virDomainDeviceDefCopy.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 617d7d5..eca54d0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8615,6 +8615,22 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
goto cleanup;
+if (flags & VIR_DOMAIN_AFFECT_CONFIG &&
+flags & VIR_DOMAIN_AFFECT_LIVE) {
+devLive = virDomainDeviceDefParse(xml, vm->def,
+ caps, driver->xmlopt,
+ parse_flags);
+if (!devLive)
+goto clean
Re: [libvirt] [PATCH v1 1/3] adding unplug_timeout QEMU conf
On Mon, Aug 19, 2019 at 11:16:24AM +0200, Ján Tomko wrote: > On Sun, Aug 18, 2019 at 06:45:29PM -0300, Daniel Henrique Barboza wrote: > > For some architectures and setups, device removal can take > > longer than the default 5 seconds. This results in commands > > such as 'virsh setvcpus' to fire timeout messages even if > > the actual operation happened in the guest, causing confusion > > for the user. > > > > The commit that introduced this error message: >commit e3229f6e4461cd1721dc68a32e16ab1718ae716e >qemu: hotplug: Add support for VCPU unplug > > specifically says that we treat this differently than regular device > detach: > >As the new code is using device_del all the implications of using it >are present. Contrary to the device deletion code, the vcpu deletion >code fails if the unplug request is not executed in time. > > Technically, we already did execute the unplug request so we lie to the > user saying "operation failed". > > Maybe we can revisit the decision? [cc-ing pkrempa who added this] > > > This patch adds a new qemu.conf parameter called 'unplug_timeout' > > to handle these cases. If left unset, the current default > > timeout is used. To avoid user 'experimentation' with small > > timeouts, the current timeout is also the minimal value > > allowed. > > > > The reason for this timeout is that we originally promised something > that we cannot deliver - a synchronous device detach API, while the > operation itself is asynchronous. I'm not a fan of exposing it and > making it configurable. I'm especially *not* a fan because the commit messages says this is a problem on certain architectures. Since we know what those arches are, we should use a larger timeout for those arches out of the box. Requiring admin to set a config param to fix the architectures is super unpleasant out of the box experiance. 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] [RFC] On present using dummy hostdev usb device
Hi, all! We use an interesting approach when starting/migrating/etc domain with usb hostdev with startupPolicy=optional. We add qemu usb-host device with missing hostaddr/hostbus parameters (dummy device). I guess there are 2 reasons why we do it. First without dummy device migration will fail as described in [1]. Second is an interesting property of dummy device that qemu starts to monitor for attaching of usb devices and binds the first attached to node to the dummy device. So one can start a domain with missing hostdev and attach it later or migrate a domain then detach hostdev on source and attach it on destination. But as qemu binds the first attached device this is not reliable, to say the least. And after all this does not work if domain uses distinct mount namespace which is default. So I question does it make sense to use dummy device at all? In case of migration/resume from suspend/revert to snapshot we can either fix qemu to ignore incoming missing hostdev data or add dummy device temporarily. The latter solution is worse as it brings dummy device behaviour even for a short period of time. However having a temporary dummy device is neccessary step towards the time when all supported versions of qemu do the mentioned ignoring. As to handling attaching of missing hostdev device to node it should be done in libvirt which can do necessary mount namespace actions. (Actually I developing such patches right now but some peculiarities of dummy device bring me here). Nikolay [1] https://www.redhat.com/archives/libvir-list/2012-October/msg00440.html -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] selinux: Do not report an error when not returning -1
On 8/29/19 5:39 PM, Martin Kletzander wrote: I guess the reason for that was the automatic interpretation/stringification of setfilecon_errno, but the code was not nice to read and it was a bit confusing. Also, the logs and error states get cleaner this way. Signed-off-by: Martin Kletzander --- I'm still waiting for the build in travis to finish, so don't stone me if it fails. The link is here: https://travis-ci.org/nertpinx/libvirt/builds/578418517 src/security/security_selinux.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) Reviewed-by: Michal Privoznik And safe for freeze. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] On present using dummy hostdev usb device
On Fri, Aug 30, 2019 at 08:44:03AM +, Nikolay Shirokovskiy wrote: > Hi, all! > > We use an interesting approach when starting/migrating/etc domain with usb > hostdev with startupPolicy=optional. We add qemu usb-host device with > missing hostaddr/hostbus parameters (dummy device). I guess there are > 2 reasons why we do it. First without dummy device migration will fail as > described in [1]. Second is an interesting property of dummy device that > qemu starts to monitor for attaching of usb devices and binds the first > attached to node to the dummy device. So one can start a domain with > missing hostdev and attach it later or migrate a domain then detach > hostdev on source and attach it on destination. But as qemu binds the > first attached device this is not reliable, to say the least. And after > all this does not work if domain uses distinct mount namespace which > is default. Even without mount namespaces, it should fail as QEMU is running non-root and libvirt won't have granted access to any host USB devices in /dev, and also SELinux policy will forbid this. > So I question does it make sense to use dummy device at all? In case of > migration/resume from suspend/revert to snapshot we can either fix qemu to > ignore incoming missing hostdev data or add dummy device temporarily. The > latter solution is worse as it brings dummy device behaviour even for a short > period of time. However having a temporary dummy device is neccessary step > towards the time when all supported versions of qemu do the mentioned > ignoring. > As to handling attaching of missing hostdev device to node it should be done > in > libvirt which can do necessary mount namespace actions. (Actually I developing > such patches right now but some peculiarities of dummy device bring me here). The problems around host USB device passthrough are conceptually similar to the problems of hots PCI device passthrough. In both cases we cannot assume the device present on the source device exists on the target device in the same way. In both cases, even if the device does exist on the target, we cannot serialize the state of the host device across the migration. For PCI devices we simply refuse to initiate the migration if any host PCI devices are attached. The mgmt app has to hot-unplug all devices before migration, and hot-plug new devices after migration if desired. I'm inclined to suggest that same approach of hotunplug + hotplug either side of migration is the only viable option for host USB devices too. As such any mgmt app could do this dance today without any changes in libvirt. If we turned host USB devices into a migration blocker though, that could be considered a significant change of behaviour for mgmt apps, even though this dummy USB device is effectively useless due to our security policies. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 1/3] adding unplug_timeout QEMU conf
On 8/30/19 5:24 AM, Daniel P. Berrangé wrote: On Mon, Aug 19, 2019 at 11:16:24AM +0200, Ján Tomko wrote: On Sun, Aug 18, 2019 at 06:45:29PM -0300, Daniel Henrique Barboza wrote: For some architectures and setups, device removal can take longer than the default 5 seconds. This results in commands such as 'virsh setvcpus' to fire timeout messages even if the actual operation happened in the guest, causing confusion for the user. The commit that introduced this error message: commit e3229f6e4461cd1721dc68a32e16ab1718ae716e qemu: hotplug: Add support for VCPU unplug specifically says that we treat this differently than regular device detach: As the new code is using device_del all the implications of using it are present. Contrary to the device deletion code, the vcpu deletion code fails if the unplug request is not executed in time. Technically, we already did execute the unplug request so we lie to the user saying "operation failed". Maybe we can revisit the decision? [cc-ing pkrempa who added this] This patch adds a new qemu.conf parameter called 'unplug_timeout' to handle these cases. If left unset, the current default timeout is used. To avoid user 'experimentation' with small timeouts, the current timeout is also the minimal value allowed. The reason for this timeout is that we originally promised something that we cannot deliver - a synchronous device detach API, while the operation itself is asynchronous. I'm not a fan of exposing it and making it configurable. I'm especially *not* a fan because the commit messages says this is a problem on certain architectures. Since we know what those arches are, we should use a larger timeout for those arches out of the box. Requiring admin to set a config param to fix the architectures is super unpleasant out of the box experiance. Good point. I'll re-send the series changing the timeout for PowerPC guests only. There's no need to impact all users for a problem that so far only impacts PPC. Thanks, DHB Regards, Daniel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH trivial] qemu_conf.c: removing unused virQEMUDriverConfigPtr variable
'virQEMUDriverConfigPtr cfg' is declared, initiated, but never
used in virQEMUDriverCreateCapabilities().
Signed-off-by: Daniel Henrique Barboza
---
Accidental discovery while reading code.
src/qemu/qemu_conf.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 89c183e46a..d771bb6916 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1266,7 +1266,6 @@ virCapsPtr
virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver)
virSecurityManagerPtr *sec_managers = NULL;
/* Security driver data */
const char *doi, *model, *lbl, *type;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
const int virtTypes[] = {VIR_DOMAIN_VIRT_KVM,
VIR_DOMAIN_VIRT_QEMU,};
@@ -1313,13 +1312,11 @@ virCapsPtr
virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver)
}
VIR_FREE(sec_managers);
-virObjectUnref(cfg);
return caps;
error:
VIR_FREE(sec_managers);
virObjectUnref(caps);
-virObjectUnref(cfg);
return NULL;
}
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH 2/2] qapi: deprecate implicit filters
John Snow writes: > On 8/29/19 12:45 PM, Christophe de Dinechin wrote: >> [...] >> Sorry for catching up late, this mail thread happened during my PTO. >> >> I remember bringing up at the time [1] that the correct solution needs >> to take into account usage models that vary from >> >> - a workstation case, where displaying an error box is easy and >> convenient, >> >> - to local headless VMs where system-level notification would do the job >> better, allowing us to leverage things like system-wide email notifications >> >> - to large-scale collections of VMs managed by some layered product, >> where the correct reporting would be through something like Insights, >> i.e. you don't scan individual logs, you want something like "913 VMs >> are using deprecated X" >> >> To me, that implies that we need to have a clear division of roles, with >> a standard way to >> >> a) produce the errors, >> b) propagate them, > > I started replying to this thread to the other mail you sent; I think > this is going to be fairly involved. I wouldn't mind being proven wrong > though. Yes, I think it does look involved, but mostly for historical reasons. In other words, what is complicated is preserving the historical behaviors so as to not break existing consumers. > >> c) consume them (at least up to libvirt) >> >> Notice that this work has already been done for "real" errors, >> i.e. there is a real QAPI notion of "errors". AFAICT, warn_report does >> not connect to it, though, it goes through error_vprintf which is really >> just basic logging. >> >> So would it make sense to: >> >> 1. Add a deprecation_report() alongside warn_report()? >> > > Where's that get routed to? just an error_vprintf style situation? Yes, but see below. > >> 2. Connect warn_report() and all the error_vprintf output to QAPI, >>e.g. using John's suggestion of adding the messages using some >>"warning" or "deprecated" tag? >> > > How do you correlate them? Without having looked at the code much, I think I would 1. extend the existing QAPI error to support warnings, deprecations and info messages. The first problem I see is that there is no error, so we may sometimes need to create one when there was none before. And of course make sure that this does not ultimately show as an error, but as a success with additional annotations. 2. replace the current "link + if" switching for error_vprintf with some actual notification mechanism, with one option routine to monitor_vprintf, one to stderr, one to log file, and then an additional path that would post a newly minted qapi warning. > >> 3. Teach libvirt how to consume that new tag and pass it along? >> > > I think it's not libvirt's job to pass it along, exactly -- libvirt made > the decision for which features to engage in QEMU, not the end user. First, by "pass along", I meant to possible layered products or management software. We don't necessarily need a new virErrorLevel, deprecation could be a warning with some special domain, e.g. VIR_FROM_DEPRECATION. There may be a need to add some API here. Looking at the code, it's not obvious to me that libvirt has any notion of error priority. In other words, if you raise an error then a warning, you get the warning as the last error, right? Second, why not report the use of deprecated features? I don't fully buy the rationale that libvirt engages the features, because it does not do it on its own, it does it because the user made some specific request. This point of view also seems to require that libvirt or the user should know ahead of time it's about to engage a deprecated feature. To me, the problem is precisely that neither libvirt nor the user knows, which is why we are discussing how to best make it known. > > If the user upgrades QEMU but not libvirt, it's not really anything they > have control over and they shouldn't be pestered with such things. > > However, if libvirt accidentally released a version that engages > deprecated behavior (and were unaware of it), it'd be nice to get user > reports, surely? > > Logging messages for libvirt might be the best that can be done there in > that case. I personally would treat that like any warning. > > > In contrast, power user tools like QMP libraries, qmp-shell and others > allow more direct and meaningful access to QMP, so those should report > deprecation messages to the user. Agreed. > >> >> [1] https://lists.nongnu.org/archive/html/qemu-devel/2018-10/msg06131.html >> -- Thanks, Christophe de Dinechin (IRC c3d) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH trivial] qemu_conf.c: removing unused virQEMUDriverConfigPtr variable
On Fri, Aug 30, 2019 at 07:03:55AM -0300, Daniel Henrique Barboza wrote: > 'virQEMUDriverConfigPtr cfg' is declared, initiated, but never > used in virQEMUDriverCreateCapabilities(). > > Signed-off-by: Daniel Henrique Barboza > --- Reviewed-by: Erik Skultety and safe for freeze -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH trivial] qemu_conf.c: removing unused virQEMUDriverConfigPtr variable
Daniel Henrique Barboza writes:
> 'virQEMUDriverConfigPtr cfg' is declared, initiated, but never
> used in virQEMUDriverCreateCapabilities().
>
> Signed-off-by: Daniel Henrique Barboza
> ---
>
> Accidental discovery while reading code.
>
> src/qemu/qemu_conf.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 89c183e46a..d771bb6916 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -1266,7 +1266,6 @@ virCapsPtr
> virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver)
> virSecurityManagerPtr *sec_managers = NULL;
> /* Security driver data */
> const char *doi, *model, *lbl, *type;
> -virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
> const int virtTypes[] = {VIR_DOMAIN_VIRT_KVM,
> VIR_DOMAIN_VIRT_QEMU,};
>
> @@ -1313,13 +1312,11 @@ virCapsPtr
> virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver)
> }
> VIR_FREE(sec_managers);
>
> -virObjectUnref(cfg);
> return caps;
>
> error:
> VIR_FREE(sec_managers);
> virObjectUnref(caps);
> -virObjectUnref(cfg);
> return NULL;
> }
>
> --
> 2.21.0
Reviewed-by: Christophe de Dinechin
--
Cheers,
Christophe de Dinechin (IRC c3d)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 1/3] adding unplug_timeout QEMU conf
Daniel P. Berrangé writes: >> > >> >> The reason for this timeout is that we originally promised something >> that we cannot deliver - a synchronous device detach API, while the >> operation itself is asynchronous. I'm not a fan of exposing it and >> making it configurable. > > I'm especially *not* a fan because the commit messages says this is > a problem on certain architectures. Since we know what those arches > are, we should use a larger timeout for those arches out of the box. > Requiring admin to set a config param to fix the architectures is > super unpleasant out of the box experiance. True, but also notice that 5 seconds is also already close to the attention span time limit for users [1]. So increasing it to 10s might bring people to believe things are stuck, unless you provide some sort of feedback that this is normal. https://www.nngroup.com/articles/response-times-3-important-limits/ > > > 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 :| -- Cheers, Christophe de Dinechin (IRC c3d) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 2/4] qemu: guestinfo: handle unsupported agent commands
On 8/27/19 4:35 PM, Jonathon Jongsma wrote:
> When we're collecting guest information, older agents may not support
> all agent commands. In the case where the user requested all info
> types (i.e. types == 0), ignore unsupported command errors and gather as
> much information as possible. If the agent command failed for some other
> reason, or if the user explciitly requested a specific info type (i.e.
> types != 0), abort on the first error.
>
> Signed-off-by: Jonathon Jongsma
> ---
> src/qemu/qemu_agent.c | 70 ++
> src/qemu/qemu_driver.c | 33 ++--
> tests/qemuagenttest.c | 2 +-
> 3 files changed, 82 insertions(+), 23 deletions(-)
>
> diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
> index f2a8bb6263..c63db968c6 100644
> --- a/src/qemu/qemu_agent.c
> +++ b/src/qemu/qemu_agent.c
> @@ -995,6 +995,26 @@ qemuAgentStringifyErrorClass(const char *klass)
> return "unknown QEMU command error";
> }
>
> +/* Checks whether the agent reply msg is an error caused by an unsupported
> + * command.
> + *
> + * Returns true when reply is CommandNotFound or CommandDisabled
> + * false otherwise
> + */
> +static bool
> +qemuAgentErrorCommandUnsupported(virJSONValuePtr reply)
> +{
> +const char *klass;
> +virJSONValuePtr error = virJSONValueObjectGet(reply, "error");> +
Coverity notes it's possible to enter this function with @reply ==
NULL... Calls to qemuAgentCommand will set @*reply = NULL immediately
and only fill in @*reply when/if qemuAgentSend returns 0 *and*
@msg.rxObject != NULL.
When virJSONValueObjectGet is called it derefs it's first parameter
(@object) immediately
John
[...]
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 09/16] qemu: add vhost-user helpers
Hi
On Tue, Aug 27, 2019 at 12:30 AM Cole Robinson wrote:
>
> On 8/23/19 12:21 PM, Cole Robinson wrote:
> > From: Marc-André Lureau
> >
> > Add qemuVhostUserFetchConfigs() to discover vhost-user helpers.
> >
> > qemuVhostUserFillDomainGPU() will find the first matching GPU helper
> > with the required capabilities and set the associated
> > vhost_user_binary.
> >
> > Signed-off-by: Marc-André Lureau
> > Signed-off-by: Cole Robinson
> > ---
> > src/conf/device_conf.c| 1 +
> > src/conf/device_conf.h| 1 +
> > src/qemu/Makefile.inc.am | 2 +
> > src/qemu/qemu_vhost_user.c| 394 ++
> > src/qemu/qemu_vhost_user.h| 48 +++
> > tests/Makefile.am | 9 +
> > .../etc/qemu/vhost-user/40-gpu.json | 1 +
> > .../etc/qemu/vhost-user/50-gpu.json | 0
> > .../qemu/vhost-user/test-vhost-user-gpu | 11 +
> > .../usr/share/qemu/vhost-user/30-gpu.json | 1 +
> > .../usr/share/qemu/vhost-user/50-gpu.json | 8 +
> > .../usr/share/qemu/vhost-user/60-gpu.json | 1 +
> > tests/qemuvhostusertest.c | 132 ++
> > 13 files changed, 609 insertions(+)
> > create mode 100644 src/qemu/qemu_vhost_user.c
> > create mode 100644 src/qemu/qemu_vhost_user.h
> > create mode 12 tests/qemuvhostuserdata/etc/qemu/vhost-user/40-gpu.json
> > create mode 100644 tests/qemuvhostuserdata/etc/qemu/vhost-user/50-gpu.json
> > create mode 100755
> > tests/qemuvhostuserdata/usr/libexec/qemu/vhost-user/test-vhost-user-gpu
> > create mode 12
> > tests/qemuvhostuserdata/usr/share/qemu/vhost-user/30-gpu.json
> > create mode 100644
> > tests/qemuvhostuserdata/usr/share/qemu/vhost-user/50-gpu.json
> > create mode 12
> > tests/qemuvhostuserdata/usr/share/qemu/vhost-user/60-gpu.json
> > create mode 100644 tests/qemuvhostusertest.c
> >
> > diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
> > index 4c57f0995f..2f7077ddc4 100644
> > --- a/src/conf/device_conf.c
> > +++ b/src/conf/device_conf.c
> > @@ -96,6 +96,7 @@ virDomainDeviceInfoClear(virDomainDeviceInfoPtr info)
> > VIR_FREE(info->loadparm);
> > info->isolationGroup = 0;
> > info->isolationGroupLocked = false;
> > +VIR_FREE(info->vhost_user_binary);
> > }
> >
> > void
> > diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
> > index d0854925e3..0b0c525ad8 100644
> > --- a/src/conf/device_conf.h
> > +++ b/src/conf/device_conf.h
> > @@ -179,6 +179,7 @@ struct _virDomainDeviceInfo {
> > * cases we might want to prevent that from happening by
> > * locking the isolation group */
> > bool isolationGroupLocked;
> > +char *vhost_user_binary;
> > };
> >
> > void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info);
> > diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
> > index f7a0fa4a84..18a9220d01 100644
> > --- a/src/qemu/Makefile.inc.am
> > +++ b/src/qemu/Makefile.inc.am
> > @@ -60,6 +60,8 @@ QEMU_DRIVER_SOURCES = \
> > qemu/qemu_qapi.h \
> > qemu/qemu_tpm.c \
> > qemu/qemu_tpm.h \
> > + qemu/qemu_vhost_user.c \
> > + qemu/qemu_vhost_user.h \
> > $(NULL)
> >
> >
> > diff --git a/src/qemu/qemu_vhost_user.c b/src/qemu/qemu_vhost_user.c
> > new file mode 100644
> > index 00..7a97052ab9
> > --- /dev/null
> > +++ b/src/qemu/qemu_vhost_user.c
> > @@ -0,0 +1,394 @@
> > +/*
> > + * qemu_vhost_user.c: QEMU vhost-user
> > + *
> > + * Copyright (C) 2019 Red Hat, Inc.
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library. If not, see
> > + *
Re: [libvirt] [PATCH v2 09/16] qemu: add vhost-user helpers
Hi
On Tue, Aug 27, 2019 at 12:29 PM Ján Tomko wrote:
>
> On Fri, Aug 23, 2019 at 12:21:53PM -0400, Cole Robinson wrote:
> >From: Marc-André Lureau
> >
> >Add qemuVhostUserFetchConfigs() to discover vhost-user helpers.
> >
> >qemuVhostUserFillDomainGPU() will find the first matching GPU helper
> >with the required capabilities and set the associated
> >vhost_user_binary.
> >
>
> Looks good, but it seems like something we might want to start caching
> over time, especially if more than one helper per domain will be needed.
>
> >Signed-off-by: Marc-André Lureau
> >Signed-off-by: Cole Robinson
> >---
> > src/conf/device_conf.c| 1 +
> > src/conf/device_conf.h| 1 +
> > src/qemu/Makefile.inc.am | 2 +
> > src/qemu/qemu_vhost_user.c| 394 ++
> > src/qemu/qemu_vhost_user.h| 48 +++
> > tests/Makefile.am | 9 +
> > .../etc/qemu/vhost-user/40-gpu.json | 1 +
> > .../etc/qemu/vhost-user/50-gpu.json | 0
> > .../qemu/vhost-user/test-vhost-user-gpu | 11 +
> > .../usr/share/qemu/vhost-user/30-gpu.json | 1 +
> > .../usr/share/qemu/vhost-user/50-gpu.json | 8 +
> > .../usr/share/qemu/vhost-user/60-gpu.json | 1 +
> > tests/qemuvhostusertest.c | 132 ++
> > 13 files changed, 609 insertions(+)
> > create mode 100644 src/qemu/qemu_vhost_user.c
> > create mode 100644 src/qemu/qemu_vhost_user.h
> > create mode 12 tests/qemuvhostuserdata/etc/qemu/vhost-user/40-gpu.json
> > create mode 100644 tests/qemuvhostuserdata/etc/qemu/vhost-user/50-gpu.json
> > create mode 100755
> > tests/qemuvhostuserdata/usr/libexec/qemu/vhost-user/test-vhost-user-gpu
> > create mode 12
> > tests/qemuvhostuserdata/usr/share/qemu/vhost-user/30-gpu.json
> > create mode 100644
> > tests/qemuvhostuserdata/usr/share/qemu/vhost-user/50-gpu.json
> > create mode 12
> > tests/qemuvhostuserdata/usr/share/qemu/vhost-user/60-gpu.json
> > create mode 100644 tests/qemuvhostusertest.c
> >
> >diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
> >index 4c57f0995f..2f7077ddc4 100644
> >--- a/src/conf/device_conf.c
> >+++ b/src/conf/device_conf.c
> >@@ -96,6 +96,7 @@ virDomainDeviceInfoClear(virDomainDeviceInfoPtr info)
> > VIR_FREE(info->loadparm);
> > info->isolationGroup = 0;
> > info->isolationGroupLocked = false;
> >+VIR_FREE(info->vhost_user_binary);
> > }
> >
> > void
> >diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
> >index d0854925e3..0b0c525ad8 100644
> >--- a/src/conf/device_conf.h
> >+++ b/src/conf/device_conf.h
> >@@ -179,6 +179,7 @@ struct _virDomainDeviceInfo {
> > * cases we might want to prevent that from happening by
> > * locking the isolation group */
> > bool isolationGroupLocked;
> >+char *vhost_user_binary;
> > };
> >
>
> Not sure about the placement in DeviceInfo, looks more fit for 'driver'
> to me.
>
That wouldn't follow firmware code, but other than that, driver
doesn't really fit either since it's a per-domain/device value.
--
Marc-André Lureau
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/6] qemu: handle libvirtd restarts during hostdev reattaching
We need to handle case when libvirtd is restarted after qemu device is
deleted in order to reattach hostdev and before device deleted event is
delivired. First we need to save delete_cause in status xml so that if
event is delivered after libvirtd started back we can proceed with
reattaching. Second if we missed the event we need to continue with
reattaching too.
Signed-off-by: Nikolay Shirokovskiy
---
src/conf/domain_conf.c | 26 ++
src/conf/domain_conf.h | 1 +
src/qemu/qemu_driver.c | 20 ++--
src/qemu/qemu_process.c | 6 ++
4 files changed, 47 insertions(+), 6 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9a2385e31c..ecab83147e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1236,6 +1236,13 @@ VIR_ENUM_IMPL(virDomainShmemModel,
"ivshmem-doorbell",
);
+VIR_ENUM_IMPL(virDomainHostdevDeleteCause,
+ VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_LAST,
+ "none",
+ "client",
+ "reattaching"
+);
+
VIR_ENUM_IMPL(virDomainLaunchSecurity,
VIR_DOMAIN_LAUNCH_SECURITY_LAST,
"",
@@ -7534,6 +7541,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
VIR_AUTOFREE(char *) startupPolicy = NULL;
VIR_AUTOFREE(char *) autoAddress = NULL;
VIR_AUTOFREE(char *) missing = NULL;
+VIR_AUTOFREE(char *) deleteCause = NULL;
if ((startupPolicy = virXMLPropString(node, "startupPolicy"))) {
def->startupPolicy =
@@ -7556,6 +7564,18 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
def->missing = true;
}
+if ((deleteCause = virXMLPropString(node, "deleteCause"))) {
+def->deleteCause =
+virDomainHostdevDeleteCauseTypeFromString(deleteCause);
+
+if (def->deleteCause <= 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("Unknown deleteCause '%s'"),
+ deleteCause);
+goto out;
+}
+}
+
/* Product can validly be 0, so we need some extra help to determine
* if it is uninitialized*/
got_product = false;
@@ -24911,6 +24931,12 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf,
if (def->missing && !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))
virBufferAddLit(buf, " missing='yes'");
+
+if (def->deleteCause && (flags & VIR_DOMAIN_DEF_FORMAT_STATUS)) {
+const char *deleteCause;
+deleteCause =
virDomainHostdevDeleteCauseTypeToString(def->deleteCause);
+virBufferAsprintf(buf, " deleteCause='%s'", deleteCause);
+}
}
if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI &&
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 49392d0286..a2de4c7657 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -333,6 +333,7 @@ typedef enum {
VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_LAST
} virDomainHostdevDeleteCauseType;
+VIR_ENUM_DECL(virDomainHostdevDeleteCause);
/* basic device for direct passthrough */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 352f6d0376..b912766445 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5265,11 +5265,14 @@ processUSBAddedEvent(virQEMUDriverPtr driver,
struct qemuUdevUSBAddData *data)
{
virDomainDeviceDef dev = { .type = VIR_DOMAIN_DEVICE_HOSTDEV };
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
virDomainHostdevDefPtr hostdev;
size_t i;
-if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) {
+virObjectUnref(cfg);
return;
+}
if (!virDomainObjIsActive(vm)) {
VIR_DEBUG("Domain is not running");
@@ -5295,8 +5298,13 @@ processUSBAddedEvent(virQEMUDriverPtr driver,
if (qemuDomainDetachDeviceLive(vm, &dev, driver, true, true) < 0)
goto cleanup;
+if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) <
0)
+VIR_WARN("unable to save domain status after reattaching device %s",
+ hostdev->info->alias);
+
cleanup:
qemuDomainObjEndJob(driver, vm);
+virObjectUnref(cfg);
}
@@ -5307,10 +5315,13 @@ processUSBRemovedEvent(virQEMUDriverPtr driver,
{
size_t i;
virDomainDeviceDef dev = { .type = VIR_DOMAIN_DEVICE_HOSTDEV };
+virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
virDomainHostdevDefPtr hostdev;
-if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) {
+virObjectUnref(cfg);
return;
+}
if (!virDomainObjIsActive(vm)) {
VIR_DEBUG("Domain is not running");
@@ -5335,8 +5346,13 @@ processUSBRemovedEvent(virQEMUDriverPtr driver,
if (qemuDomainDetachDeviceLive(vm, &dev, driver, true, true) < 0)
[libvirt] [PATCH 2/6] qemu: handle usb hostdev add/del udev events
In either case detach qemu hostdev device (it is dummy in case
node hostdev is absent). Meaningful processing will be in DEVICE_DELETED
event handler.
Signed-off-by: Nikolay Shirokovskiy
---
src/qemu/Makefile.inc.am | 2 +
src/qemu/qemu_conf.h | 3 +
src/qemu/qemu_domain.c | 2 +
src/qemu/qemu_domain.h | 2 +
src/qemu/qemu_driver.c | 344 ++-
5 files changed, 352 insertions(+), 1 deletion(-)
diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
index d16b315ebc..8be0dee396 100644
--- a/src/qemu/Makefile.inc.am
+++ b/src/qemu/Makefile.inc.am
@@ -85,6 +85,7 @@ libvirt_driver_qemu_impl_la_CFLAGS = \
-I$(srcdir)/conf \
-I$(srcdir)/secret \
$(AM_CFLAGS) \
+ $(UDEV_CFLAGS) \
$(NULL)
libvirt_driver_qemu_impl_la_LDFLAGS = $(AM_LDFLAGS)
libvirt_driver_qemu_impl_la_LIBADD = \
@@ -93,6 +94,7 @@ libvirt_driver_qemu_impl_la_LIBADD = \
$(LIBNL_LIBS) \
$(SELINUX_LIBS) \
$(LIBXML_LIBS) \
+ $(UDEV_LIBS) \
$(NULL)
libvirt_driver_qemu_impl_la_SOURCES = $(QEMU_DRIVER_SOURCES)
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 0cbddd7a9c..2e50bb0950 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -294,6 +294,9 @@ struct _virQEMUDriver {
/* Immutable pointer, self-locking APIs */
virHashAtomicPtr migrationErrors;
+
+struct udev_monitor *udev_monitor;
+int udev_watch;
};
virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 657f3ecfe4..4784804d1e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -15034,6 +15034,8 @@ qemuProcessEventFree(struct qemuProcessEvent *event)
case QEMU_PROCESS_EVENT_SERIAL_CHANGED:
case QEMU_PROCESS_EVENT_BLOCK_JOB:
case QEMU_PROCESS_EVENT_MONITOR_EOF:
+case QEMU_PROCESS_EVENT_USB_REMOVED:
+case QEMU_PROCESS_EVENT_USB_ADDED:
VIR_FREE(event->data);
break;
case QEMU_PROCESS_EVENT_JOB_STATUS_CHANGE:
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index d097f23342..94aea62693 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -521,6 +521,8 @@ typedef enum {
QEMU_PROCESS_EVENT_MONITOR_EOF,
QEMU_PROCESS_EVENT_PR_DISCONNECT,
QEMU_PROCESS_EVENT_RDMA_GID_STATUS_CHANGED,
+QEMU_PROCESS_EVENT_USB_REMOVED,
+QEMU_PROCESS_EVENT_USB_ADDED,
QEMU_PROCESS_EVENT_LAST
} qemuProcessEventType;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2378a2e7d0..33b75a3c71 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -34,6 +34,7 @@
#include
#include
#include
+#include
#include "qemu_driver.h"
@@ -719,6 +720,254 @@ qemuDomainFindMaxID(virDomainObjPtr vm,
}
+struct qemuUdevUSBRemoveData {
+unsigned int bus;
+unsigned int device;
+};
+
+struct qemuUdevUSBAddData {
+unsigned int vendor;
+unsigned int product;
+};
+
+struct qemuUdevUSBEventData {
+union {
+struct qemuUdevUSBRemoveData remove;
+struct qemuUdevUSBAddData add;
+} data;
+bool found;
+bool remove;
+};
+
+static int
+qemuUdevUSBHandleEvent(virDomainObjPtr vm, void *opaque)
+{
+struct qemuUdevUSBEventData *data = opaque;
+struct qemuProcessEvent *event = NULL;
+size_t i;
+
+if (data->found)
+return 0;
+
+virObjectLock(vm);
+
+if (!virDomainObjIsActive(vm))
+goto cleanup;
+
+for (i = 0; i < vm->def->nhostdevs; i++) {
+virDomainHostdevDefPtr hostdev = vm->def->hostdevs[i];
+virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb;
+
+if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
+continue;
+
+if (data->remove) {
+if (usbsrc->bus != data->data.remove.bus ||
+usbsrc->device != data->data.remove.device)
+continue;
+} else {
+if (usbsrc->vendor != data->data.add.vendor ||
+usbsrc->product != data->data.add.product)
+continue;
+}
+
+data->found = true;
+
+if (VIR_ALLOC(event) < 0)
+goto cleanup;
+
+if (data->remove) {
+struct qemuUdevUSBRemoveData *rm_data;
+
+
+if (VIR_ALLOC(rm_data) < 0)
+goto cleanup;
+
+*rm_data = data->data.remove;
+event->data = rm_data;
+event->eventType = QEMU_PROCESS_EVENT_USB_REMOVED;
+} else {
+struct qemuUdevUSBAddData *add_data;
+
+if (VIR_ALLOC(add_data) < 0)
+goto cleanup;
+
+*add_data = data->data.add;
+event->data = add_data;
+event->eventType = QEMU_PROCESS_EVENT_USB_ADDED;
+}
+
+event->vm = virObjectRef(vm);
+
+if (virThreadPoolSendJob(qemu_driver->workerPool, 0, event) < 0) {
+virObjectUnref(vm);
+
[libvirt] [PATCH 0/6] hostdev: handle usb detach/attach on node
Hi, all. This patch series aims to handle case when hostdev usb is detached and then attached back on node. Currently libvirtd does not track these events so that in the result hostdev usb is not usable. On the first glace it make sense to delete device from qemu when hostdev is detached on node and then add device back when hostdev is attached back. But these series adds a different behaviour. On detaching device is deleted from qemu and dummy (missing hostaddr and hostbus values) hostdev is inserted, then on attaching device on node dummy device is deleted and proper one is inserted. This way we have the same semantics of 'missing' flag for hostdev as in case of hostdev missing on startup/migration to different host. Also with such approach we additionally handle cases when hostdev is missing on startup but later is attached to node. Known issues: The last patch tries to handle case when hostdev attached/detached when libvirtd is down but if fails in one scenario. If hostdev is detached and thus dummy device in inserted in qemu then if libvirtd is down and hostdev is attached to node then qemu tries to bind appeared hostdev to the dummy device but fails as device is missing in qemu's mount namespace. Later when libvirtd is up and tries to replace dummy device with actual one the device_add fails because libusb the qemu uses tracks that deviced was missed ealier. For this reason I proposed to get rid of dummy device in RFC [1]. Alternatively libusb can be fixed. [1] https://www.redhat.com/archives/libvir-list/2019-August/msg01396.html Nikolay Shirokovskiy (6): qemu: track hostdev delete cause qemu: handle usb hostdev add/del udev events qemu: handle usb hostdev add/del on device DELETED_EVENT conf: parse hostdev missing flag qemu: handle libvirtd restarts during hostdev reattaching qemu: handle hostdev add/del when libvirtd is down src/conf/domain_conf.c | 32 src/conf/domain_conf.h | 10 ++ src/qemu/Makefile.inc.am | 2 + src/qemu/qemu_conf.h | 3 + src/qemu/qemu_domain.c | 2 + src/qemu/qemu_domain.h | 2 + src/qemu/qemu_driver.c | 376 +-- src/qemu/qemu_hotplug.c | 94 -- src/qemu/qemu_hotplug.h | 7 +- src/qemu/qemu_process.c | 55 +- src/util/virhostdev.c| 6 +- tests/qemuhotplugtest.c | 2 +- 12 files changed, 558 insertions(+), 33 deletions(-) -- 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/6] conf: parse hostdev missing flag
We want to keep this flag across libvirtd restarts.
Signed-off-by: Nikolay Shirokovskiy
---
src/conf/domain_conf.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b7a342bb91..9a2385e31c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7533,6 +7533,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
virDomainHostdevSubsysUSBPtr usbsrc = &def->source.subsys.u.usb;
VIR_AUTOFREE(char *) startupPolicy = NULL;
VIR_AUTOFREE(char *) autoAddress = NULL;
+VIR_AUTOFREE(char *) missing = NULL;
if ((startupPolicy = virXMLPropString(node, "startupPolicy"))) {
def->startupPolicy =
@@ -7550,6 +7551,11 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
usbsrc->autoAddress = true;
}
+if ((missing = virXMLPropString(node, "missing"))) {
+if (STREQ(missing, "yes"))
+def->missing = true;
+}
+
/* Product can validly be 0, so we need some extra help to determine
* if it is uninitialized*/
got_product = false;
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/6] qemu: handle usb hostdev add/del on device DELETED_EVENT
Now device DELETED_EVENT can have 2 causes. First is detaching device by
client just as before. Second is attaching/detaching hostdev assigned to
guest on node. In the latter case we reattach hostdev to guest.
When device is detached from node we remove device from qemu and insert
new dummy one with unset bus and device values and have missing flag set
to true. This is on par with case when we start domain with
startupPolicy = optional and device is missing on node.
When device is attached to node back we remove dummy device from qemu
and insert proper one with discovered bus and device and missing flag is
appropriately set to false.
Signed-off-by: Nikolay Shirokovskiy
---
src/qemu/qemu_driver.c | 12 ++-
src/qemu/qemu_hotplug.c | 73 ++---
src/qemu/qemu_hotplug.h | 4 +++
src/util/virhostdev.c | 6 ++--
4 files changed, 72 insertions(+), 23 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 33b75a3c71..352f6d0376 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4677,7 +4677,6 @@ processDeviceDeletedEvent(virQEMUDriverPtr driver,
const char *devAlias)
{
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
-virDomainDeviceDef dev;
VIR_DEBUG("Removing device %s from domain %p %s",
devAlias, vm, vm->def->name);
@@ -4690,15 +4689,10 @@ processDeviceDeletedEvent(virQEMUDriverPtr driver,
goto endjob;
}
-if (STRPREFIX(devAlias, "vcpu")) {
+if (STRPREFIX(devAlias, "vcpu"))
qemuDomainRemoveVcpuAlias(driver, vm, devAlias);
-} else {
-if (virDomainDefFindDevice(vm->def, devAlias, &dev, true) < 0)
-goto endjob;
-
-if (qemuDomainRemoveDevice(driver, vm, &dev) < 0)
-goto endjob;
-}
+else if (qemuDomainRemoveDeviceAlias(driver, vm, devAlias) < 0)
+goto endjob;
if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) <
0)
VIR_WARN("unable to save domain status after removing device %s",
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 08e60dcd0e..197b2b2fd7 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2437,8 +2437,10 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
bool teardownlabel = false;
bool teardowndevice = false;
int ret = -1;
+bool reattaching = hostdev->deleteCause ==
VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_REATTACHING;
-if (virDomainUSBAddressEnsure(priv->usbaddrs, hostdev->info) < 0)
+if (!reattaching &&
+virDomainUSBAddressEnsure(priv->usbaddrs, hostdev->info) < 0)
return -1;
if (qemuHostdevPrepareUSBDevices(driver, vm->def->name, &hostdev, 1, 0) <
0)
@@ -2463,7 +2465,7 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
if (!(devstr = qemuBuildUSBHostdevDevStr(vm->def, hostdev,
priv->qemuCaps)))
goto cleanup;
-if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs+1) < 0)
+if (!reattaching && VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs+1)
< 0)
goto cleanup;
qemuDomainObjEnterMonitor(driver, vm);
@@ -2476,7 +2478,8 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
if (ret < 0)
goto cleanup;
-vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
+if (!reattaching)
+vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
ret = 0;
cleanup:
@@ -2491,7 +2494,8 @@ qemuDomainAttachHostUSBDevice(virQEMUDriverPtr driver,
VIR_WARN("Unable to remove host device from /dev");
if (added)
qemuHostdevReAttachUSBDevices(driver, vm->def->name, &hostdev, 1);
-virDomainUSBAddressRelease(priv->usbaddrs, hostdev->info);
+if (!reattaching)
+virDomainUSBAddressRelease(priv->usbaddrs, hostdev->info);
}
VIR_FREE(devstr);
return ret;
@@ -4366,7 +4370,8 @@ qemuDomainRemoveUSBHostDevice(virQEMUDriverPtr driver,
virDomainHostdevDefPtr hostdev)
{
qemuHostdevReAttachUSBDevices(driver, vm->def->name, &hostdev, 1);
-qemuDomainReleaseDeviceAddress(vm, hostdev->info);
+if (hostdev->deleteCause != VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_REATTACHING)
+qemuDomainReleaseDeviceAddress(vm, hostdev->info);
}
static void
@@ -4408,6 +4413,7 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
char *drivealias = NULL;
char *objAlias = NULL;
bool is_vfio = false;
+bool reattaching = hostdev->deleteCause ==
VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_REATTACHING;
VIR_DEBUG("Removing host device %s from domain %p %s",
hostdev->info->alias, vm, vm->def->name);
@@ -4454,16 +4460,25 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver,
}
}
-for (i = 0; i < vm->def->nhostdevs; i++) {
-if (vm->def->hostdevs[i] == hostdev) {
-virDomainHostdevRemove(vm->def, i);
-break;
+
[libvirt] [PATCH 6/6] qemu: handle hostdev add/del when libvirtd is down
Somebody can easily detach hostdev on node while libvirtd is being
stopped. Also hostdev can be attached or detached and attached back and
so forth. Let's handle such cases.
Signed-off-by: Nikolay Shirokovskiy
---
src/qemu/qemu_process.c | 49 +
1 file changed, 49 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 5cb97de4f3..f70f0ae251 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3747,6 +3747,52 @@ qemuProcessUpdateDevices(virQEMUDriverPtr driver,
return ret;
}
+
+static int
+qemuProcessReattachUSBDevices(virQEMUDriverPtr driver,
+ virDomainObjPtr vm)
+{
+size_t i;
+
+for (i = 0; i < vm->def->nhostdevs; i++) {
+virDomainDeviceDef dev = { .type = VIR_DOMAIN_DEVICE_HOSTDEV };
+virDomainHostdevDefPtr hostdev = vm->def->hostdevs[i];
+virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb;
+bool found;
+
+
+if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
+continue;
+
+if (hostdev->missing) {
+int num;
+
+if ((num = virUSBDeviceFindByVendor(usbsrc->vendor,
usbsrc->product,
+NULL, false, NULL)) < 0)
+return -1;
+
+found = num != 0;
+} else {
+virUSBDevicePtr usb;
+
+if (virUSBDeviceFindByBus(usbsrc->bus, usbsrc->device,
+ NULL, false, &usb) < 0)
+return -1;
+
+found = usb != NULL;
+virUSBDeviceFree(usb);
+}
+
+dev.data.hostdev = hostdev;
+if ((hostdev->missing ^ !found) &&
+qemuDomainDetachDeviceLive(vm, &dev, driver, true, true) < 0)
+return -1;
+}
+
+return 0;
+}
+
+
static int
qemuDomainPerfRestart(virDomainObjPtr vm)
{
@@ -8204,6 +8250,9 @@ qemuProcessReconnect(void *opaque)
if (qemuProcessUpdateDevices(driver, obj) < 0)
goto error;
+if (qemuProcessReattachUSBDevices(driver, obj) < 0)
+goto error;
+
if (qemuRefreshPRManagerState(driver, obj) < 0)
goto error;
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/6] qemu: track hostdev delete cause
qemuDomainDetachDeviceLive is called when client asks for detaching
a device. We are going to call this function when usb device is
detached on node itself. The function itself requests qemu to detach
device from guest but as device detaching is asynchronous libvirt
config changes are done in DEVICE_DELETED event handler. As we
want to keep device in libvirt config if device detached from node
opposite to the case of client initiated detaching we need to
track the delete cause.
Signed-off-by: Nikolay Shirokovskiy
---
src/conf/domain_conf.h | 9 +
src/qemu/qemu_driver.c | 4 ++--
src/qemu/qemu_hotplug.c | 21 -
src/qemu/qemu_hotplug.h | 3 ++-
tests/qemuhotplugtest.c | 2 +-
5 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 33cef5b75c..49392d0286 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -326,6 +326,14 @@ struct _virDomainHostdevCaps {
} u;
};
+typedef enum {
+VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_NONE = 0,
+VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_CLIENT,
+VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_REATTACHING,
+
+VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_LAST
+} virDomainHostdevDeleteCauseType;
+
/* basic device for direct passthrough */
struct _virDomainHostdevDef {
@@ -343,6 +351,7 @@ struct _virDomainHostdevDef {
bool missing;
bool readonly;
bool shareable;
+virDomainHostdevDeleteCauseType deleteCause;
union {
virDomainHostdevSubsys subsys;
virDomainHostdevCaps caps;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 78f5471b79..2378a2e7d0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9123,7 +9123,7 @@ qemuDomainDetachDeviceLiveAndConfig(virQEMUDriverPtr
driver,
if (flags & VIR_DOMAIN_AFFECT_LIVE) {
int rc;
-if ((rc = qemuDomainDetachDeviceLive(vm, dev_copy, driver, false)) < 0)
+if ((rc = qemuDomainDetachDeviceLive(vm, dev_copy, driver, false,
false)) < 0)
goto cleanup;
if (rc == 0 && qemuDomainUpdateDeviceList(driver, vm,
QEMU_ASYNC_JOB_NONE) < 0)
@@ -9212,7 +9212,7 @@ qemuDomainDetachDeviceAliasLiveAndConfig(virQEMUDriverPtr
driver,
if (virDomainDefFindDevice(def, alias, &dev, true) < 0)
goto cleanup;
-if ((rc = qemuDomainDetachDeviceLive(vm, &dev, driver, true)) < 0)
+if ((rc = qemuDomainDetachDeviceLive(vm, &dev, driver, true, false)) <
0)
goto cleanup;
if (rc == 0 && qemuDomainUpdateDeviceList(driver, vm,
QEMU_ASYNC_JOB_NONE) < 0)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 63acb9c451..08e60dcd0e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -5721,7 +5721,8 @@ int
qemuDomainDetachDeviceLive(virDomainObjPtr vm,
virDomainDeviceDefPtr match,
virQEMUDriverPtr driver,
- bool async)
+ bool async,
+ bool reattaching)
{
virDomainDeviceDef detach = { .type = match->type };
virDomainDeviceInfoPtr info = NULL;
@@ -5880,6 +5881,24 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
goto cleanup;
}
+if (detach.type == VIR_DOMAIN_DEVICE_HOSTDEV) {
+virDomainHostdevDefPtr hostdev = detach.data.hostdev;
+
+/*
+ * Why having additional check in second branch? Suppose client
+ * asks for device detaching and we pass the intention to qemu
+ * but don't get DEVICE_DELETED event yet. Next USB is detached
+ * on node and we have this function called again. If we reset
+ * delete cause to 'reattaching' then device will be left in
+ * libvirt config after handling DEVICE_DELETED event while
+ * it should not as client asked to detach the device before.
+ */
+if (!reattaching)
+hostdev->deleteCause = VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_CLIENT;
+else if (hostdev->deleteCause !=
VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_CLIENT)
+hostdev->deleteCause = VIR_DOMAIN_HOSTDEV_DELETE_CAUSE_REATTACHING;
+}
+
if (async) {
ret = 0;
} else {
diff --git a/src/qemu/qemu_hotplug.h b/src/qemu/qemu_hotplug.h
index 896e6c7b98..bf812eab1a 100644
--- a/src/qemu/qemu_hotplug.h
+++ b/src/qemu/qemu_hotplug.h
@@ -116,7 +116,8 @@ int qemuDomainAttachRNGDevice(virQEMUDriverPtr driver,
int qemuDomainDetachDeviceLive(virDomainObjPtr vm,
virDomainDeviceDefPtr match,
virQEMUDriverPtr driver,
- bool async);
+ bool async,
+ bool reattaching);
void qemuDomainRemoveVcpuAlias(virQEMUDriverPtr driver,
virDomainObjPtr vm,
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index b6aad330a9..ef91d4f131
Re: [libvirt] [PATCH v2 09/16] qemu: add vhost-user helpers
On Fri, Aug 30, 2019 at 04:04:14PM +0400, Marc-André Lureau wrote:
Hi
On Tue, Aug 27, 2019 at 12:29 PM Ján Tomko wrote:
On Fri, Aug 23, 2019 at 12:21:53PM -0400, Cole Robinson wrote:
>From: Marc-André Lureau
>
>Add qemuVhostUserFetchConfigs() to discover vhost-user helpers.
>
>qemuVhostUserFillDomainGPU() will find the first matching GPU helper
>with the required capabilities and set the associated
>vhost_user_binary.
>
Looks good, but it seems like something we might want to start caching
over time, especially if more than one helper per domain will be needed.
>Signed-off-by: Marc-André Lureau
>Signed-off-by: Cole Robinson
>---
> src/conf/device_conf.c| 1 +
> src/conf/device_conf.h| 1 +
> src/qemu/Makefile.inc.am | 2 +
> src/qemu/qemu_vhost_user.c| 394 ++
> src/qemu/qemu_vhost_user.h| 48 +++
> tests/Makefile.am | 9 +
> .../etc/qemu/vhost-user/40-gpu.json | 1 +
> .../etc/qemu/vhost-user/50-gpu.json | 0
> .../qemu/vhost-user/test-vhost-user-gpu | 11 +
> .../usr/share/qemu/vhost-user/30-gpu.json | 1 +
> .../usr/share/qemu/vhost-user/50-gpu.json | 8 +
> .../usr/share/qemu/vhost-user/60-gpu.json | 1 +
> tests/qemuvhostusertest.c | 132 ++
> 13 files changed, 609 insertions(+)
> create mode 100644 src/qemu/qemu_vhost_user.c
> create mode 100644 src/qemu/qemu_vhost_user.h
> create mode 12 tests/qemuvhostuserdata/etc/qemu/vhost-user/40-gpu.json
> create mode 100644 tests/qemuvhostuserdata/etc/qemu/vhost-user/50-gpu.json
> create mode 100755
tests/qemuvhostuserdata/usr/libexec/qemu/vhost-user/test-vhost-user-gpu
> create mode 12
tests/qemuvhostuserdata/usr/share/qemu/vhost-user/30-gpu.json
> create mode 100644
tests/qemuvhostuserdata/usr/share/qemu/vhost-user/50-gpu.json
> create mode 12
tests/qemuvhostuserdata/usr/share/qemu/vhost-user/60-gpu.json
> create mode 100644 tests/qemuvhostusertest.c
>
>diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
>index 4c57f0995f..2f7077ddc4 100644
>--- a/src/conf/device_conf.c
>+++ b/src/conf/device_conf.c
>@@ -96,6 +96,7 @@ virDomainDeviceInfoClear(virDomainDeviceInfoPtr info)
> VIR_FREE(info->loadparm);
> info->isolationGroup = 0;
> info->isolationGroupLocked = false;
>+VIR_FREE(info->vhost_user_binary);
> }
>
> void
>diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
>index d0854925e3..0b0c525ad8 100644
>--- a/src/conf/device_conf.h
>+++ b/src/conf/device_conf.h
>@@ -179,6 +179,7 @@ struct _virDomainDeviceInfo {
> * cases we might want to prevent that from happening by
> * locking the isolation group */
> bool isolationGroupLocked;
>+char *vhost_user_binary;
> };
>
Not sure about the placement in DeviceInfo, looks more fit for 'driver'
to me.
That wouldn't follow firmware code, but other than that, driver
doesn't really fit either since it's a per-domain/device value.
virDomainVideoDriverDefPtr driver;
is the one I meant.
Jano
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] docs: rewrite hvsupport.html page generator in python
As part of an goal to eliminate Perl from libvirt build tools,
rewrite the hvsupport.pl tool in Python.
This was a straight conversion, manually going line-by-line to
change the syntax from Perl to Python. Thus the overall structure
of the file and approach is the same.
The new impl generates byte-for-byte identical output to the
old impl.
Signed-off-by: Daniel P. Berrangé
---
docs/Makefile.am | 6 +-
docs/hvsupport.pl | 458 -
docs/hvsupport.py | 463 ++
3 files changed, 466 insertions(+), 461 deletions(-)
delete mode 100755 docs/hvsupport.pl
create mode 100755 docs/hvsupport.py
diff --git a/docs/Makefile.am b/docs/Makefile.am
index 1cdb584b0b..f7aba5499d 100644
--- a/docs/Makefile.am
+++ b/docs/Makefile.am
@@ -194,7 +194,7 @@ EXTRA_DIST= \
$(internals_html_in) $(internals_html) $(fonts) \
$(kbase_html_in) $(kbase_html) \
aclperms.htmlinc \
- hvsupport.pl \
+ hvsupport.py \
$(schema_DATA)
acl_generated = aclperms.htmlinc
@@ -230,11 +230,11 @@ web: $(dot_html) $(internals_html) $(kbase_html) \
hvsupport.html: $(srcdir)/hvsupport.html.in
-$(srcdir)/hvsupport.html.in: $(srcdir)/hvsupport.pl $(api_DATA) \
+$(srcdir)/hvsupport.html.in: $(srcdir)/hvsupport.py $(api_DATA) \
$(top_srcdir)/src/libvirt_public.syms \
$(top_srcdir)/src/libvirt_qemu.syms $(top_srcdir)/src/libvirt_lxc.syms \
$(top_srcdir)/src/driver.h
- $(AM_V_GEN)$(PERL) $(srcdir)/hvsupport.pl $(top_srcdir)/src > $@ \
+ $(AM_V_GEN)$(PYTHON) $(srcdir)/hvsupport.py $(top_srcdir)/src > $@ \
|| { rm $@ && exit 1; }
news.html.in: \
diff --git a/docs/hvsupport.pl b/docs/hvsupport.pl
deleted file mode 100755
index 494b8a27ec..00
--- a/docs/hvsupport.pl
+++ /dev/null
@@ -1,458 +0,0 @@
-#!/usr/bin/env perl
-
-use strict;
-use warnings;
-
-use File::Find;
-
-die "syntax: $0 SRCDIR\n" unless int(@ARGV) == 1;
-
-my $srcdir = shift @ARGV;
-
-my $symslibvirt = "$srcdir/libvirt_public.syms";
-my $symsqemu = "$srcdir/libvirt_qemu.syms";
-my $symslxc = "$srcdir/libvirt_lxc.syms";
-my @drivertable = (
-"$srcdir/driver-hypervisor.h",
-"$srcdir/driver-interface.h",
-"$srcdir/driver-network.h",
-"$srcdir/driver-nodedev.h",
-"$srcdir/driver-nwfilter.h",
-"$srcdir/driver-secret.h",
-"$srcdir/driver-state.h",
-"$srcdir/driver-storage.h",
-"$srcdir/driver-stream.h",
-);
-
-my %groupheaders = (
-"virHypervisorDriver" => "Hypervisor APIs",
-"virNetworkDriver" => "Virtual Network APIs",
-"virInterfaceDriver" => "Host Interface APIs",
-"virNodeDeviceDriver" => "Host Device APIs",
-"virStorageDriver" => "Storage Pool APIs",
-"virSecretDriver" => "Secret APIs",
-"virNWFilterDriver" => "Network Filter APIs",
-);
-
-
-my @srcs;
-find({
-wanted => sub {
-if (m!$srcdir/.*/\w+_(driver|common|tmpl|monitor|hal|udev)\.c$!) {
-push @srcs, $_ if $_ !~ /vbox_driver\.c/;
-}
-}, no_chdir => 1}, $srcdir);
-
-# Map API functions to the header and documentation files they're in
-# so that we can generate proper hyperlinks to their documentation.
-#
-# The function names are grep'd from the XML output of apibuild.py.
-sub getAPIFilenames {
-my $filename = shift;
-
-my %files;
-my $line;
-
-open FILE, "<", $filename or die "cannot read $filename: $!";
-
-while (defined($line = )) {
-if ($line =~ /function name='([^']+)' file='([^']+)'/) {
-$files{$1} = $2;
-}
-}
-
-close FILE;
-
-if (keys %files == 0) {
-die "No functions found in $filename. Has the apibuild.py output
changed?";
-}
-return \%files;
-}
-
-sub parseSymsFile {
-my $apisref = shift;
-my $prefix = shift;
-my $filename = shift;
-my $xmlfilename = shift;
-
-my $line;
-my $vers;
-my $prevvers;
-
-my $filenames = getAPIFilenames($xmlfilename);
-
-open FILE, "<$filename"
-or die "cannot read $filename: $!";
-
-while (defined($line = )) {
-chomp $line;
-next if $line =~ /^\s*#/;
-next if $line =~ /^\s*$/;
-next if $line =~ /^\s*(global|local):/;
-if ($line =~ /^\s*${prefix}_(\d+\.\d+\.\d+)\s*{\s*$/) {
-if (defined $vers) {
-die "malformed syms file";
-}
-$vers = $1;
-} elsif ($line =~ /\s*}\s*;\s*$/) {
-if (defined $prevvers) {
-die "malformed syms file";
-}
-$prevvers = $vers;
-$vers = undef;
-} elsif ($line =~ /\s*}\s*${prefix}_(\d+\.\d+\.\d+)\s*;\s*$/) {
-if ($1 ne $prevvers) {
-die "malformed syms file $1 != $vers";
-}
-$prevvers = $vers;
-$vers = undef;
-} elsif ($line =~ /\s*(\w+)\s*;\s*$/) {
-$$apisref{$1} = {};
-$$apisref{$1}->{vers} = $vers;
-
Re: [libvirt] [RFC] On present using dummy hostdev usb device
On Fri, Aug 30, 2019 at 10:09:06AM +0100, Daniel P. Berrangé wrote: > On Fri, Aug 30, 2019 at 08:44:03AM +, Nikolay Shirokovskiy wrote: > > Hi, all! > > > > We use an interesting approach when starting/migrating/etc domain with usb > > hostdev with startupPolicy=optional. We add qemu usb-host device with > > missing hostaddr/hostbus parameters (dummy device). I guess there are > > 2 reasons why we do it. First without dummy device migration will fail as > > described in [1]. Second is an interesting property of dummy device that > > qemu starts to monitor for attaching of usb devices and binds the first > > attached to node to the dummy device. So one can start a domain with > > missing hostdev and attach it later or migrate a domain then detach > > hostdev on source and attach it on destination. But as qemu binds the > > first attached device this is not reliable, to say the least. And after > > all this does not work if domain uses distinct mount namespace which > > is default. > > Even without mount namespaces, it should fail as QEMU is running non-root > and libvirt won't have granted access to any host USB devices in /dev, and > also SELinux policy will forbid this. Right, but the case with mount namespaces is particularly problematic: if the device open fails due to missing device node, libusb removes the device from its internal device list. This results in the following scenario: - libvirt adds a dummy usb-host device to QEMU in place of a missing device - QEMU (via libusb) installs a watch for udev add events - the physical device is plugged into the host - QEMU detects the addition of the device and, since the dummy device matches everything, tries to open it - by this time libvirt may have not created a device node in QEMU's mount namespace, so the open fails due to missing device node, and libusb removes the device from its internal list - libvirt removes the dummy usb-host device and adds the actual usb-host device - QEMU fails to open it because it's no longer seen by libusb IOW a usb-host device with missing=true can't (reliably, because sometimes libvirt is quick enough to create the device node before QEMU gives up opening it) turn into a working one without QEMU restart. > > So I question does it make sense to use dummy device at all? In case of > > migration/resume from suspend/revert to snapshot we can either fix qemu to > > ignore incoming missing hostdev data or add dummy device temporarily. The > > latter solution is worse as it brings dummy device behaviour even for a > > short > > period of time. However having a temporary dummy device is neccessary step > > towards the time when all supported versions of qemu do the mentioned > > ignoring. > > As to handling attaching of missing hostdev device to node it should be > > done in > > libvirt which can do necessary mount namespace actions. (Actually I > > developing > > such patches right now but some peculiarities of dummy device bring me > > here). > > The problems around host USB device passthrough are conceptually similar > to the problems of hots PCI device passthrough. > > In both cases we cannot assume the device present on the source device > exists on the target device in the same way. > > In both cases, even if the device does exist on the target, we cannot > serialize the state of the host device across the migration. Right. > For PCI devices we simply refuse to initiate the migration if any host > PCI devices are attached. The mgmt app has to hot-unplug all devices > before migration, and hot-plug new devices after migration if desired. > > I'm inclined to suggest that same approach of hotunplug + hotplug either > side of migration is the only viable option for host USB devices too. > > As such any mgmt app could do this dance today without any changes in > libvirt. Are you trying to say that the mgmt app should just refrain from creating usb-host devices with missing=true? > If we turned host USB devices into a migration blocker though, that > could be considered a significant change of behaviour for mgmt apps, > even though this dummy USB device is effectively useless due to our > security policies. I'm afraid the issue is a bit more severe: the dummy device isn't just useless, it stands in the way of the real device later on. Thanks, Roman. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH for 5.7.0 3/3] qemu_blockjob: Restore seclabels more frequently on job events
If a block job reaches failed/cancelled state, or is completed
without pivot then qemu no longer uses the mirror image. Since
we've set its seclabels we must restore them back to avoid
leaking perms/XATTRs.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1741456
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_blockjob.c | 25 +++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 80302fb139..8411d8e223 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -656,6 +656,13 @@ qemuBlockJobEventProcessLegacyCompleted(virQEMUDriverPtr
driver,
} else {
if (disk->mirror) {
virDomainLockImageDetach(driver->lockManager, vm, disk->mirror);
+
+/* QEMU no longer uses the image, so we can restore its label. */
+if (qemuSecurityRestoreImageLabel(driver, vm, disk->mirror, true)
< 0) {
+VIR_WARN("Unable to restore security labels on vm %s disk %s",
+ vm->def->name, NULLSTR(disk->mirror->path));
+}
+
virObjectUnref(disk->mirror);
}
}
@@ -725,6 +732,13 @@ qemuBlockJobEventProcessLegacy(virQEMUDriverPtr driver,
case VIR_DOMAIN_BLOCK_JOB_CANCELED:
if (disk->mirror) {
virDomainLockImageDetach(driver->lockManager, vm, disk->mirror);
+
+/* QEMU no longer uses the image, so we can restore its label. */
+if (qemuSecurityRestoreImageLabel(driver, vm, disk->mirror, true)
< 0) {
+VIR_WARN("Unable to restore security labels on vm %s disk %s",
+ vm->def->name, NULLSTR(disk->mirror->path));
+}
+
virObjectUnref(disk->mirror);
disk->mirror = NULL;
}
@@ -1124,7 +1138,8 @@
qemuBlockJobProcessEventConcludedCopyAbort(virQEMUDriverPtr driver,
static void
-qemuBlockJobProcessEventFailedActiveCommit(virDomainObjPtr vm,
+qemuBlockJobProcessEventFailedActiveCommit(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
qemuBlockJobDataPtr job)
{
VIR_DEBUG("active commit job '%s' on VM '%s' failed", job->name,
vm->def->name);
@@ -1132,6 +1147,12 @@
qemuBlockJobProcessEventFailedActiveCommit(virDomainObjPtr vm,
if (!job->disk)
return;
+/* QEMU no longer uses the image, so we can restore its label. */
+if (qemuSecurityRestoreImageLabel(driver, vm, job->disk->mirror, true) <
0) {
+VIR_WARN("Unable to restore security labels on vm %s disk %s",
+ vm->def->name, NULLSTR(job->disk->mirror->path));
+}
+
virObjectUnref(job->disk->mirror);
job->disk->mirror = NULL;
}
@@ -1227,7 +1248,7 @@
qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr job,
break;
case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT:
-qemuBlockJobProcessEventFailedActiveCommit(vm, job);
+qemuBlockJobProcessEventFailedActiveCommit(driver, vm, job);
break;
case QEMU_BLOCKJOB_TYPE_CREATE:
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH for 5.7.0 1/3] qemu_blockjob: Move active commit failed state handling into a function
Currently, there are only a few lines of code so a separate
function was not necessary, but this will change. So instead of
putting all the new code under 'case
QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT' create a separate function.
Just like every other case has one.
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_blockjob.c | 19 +++
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 3003e9c518..c77a129bfc 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -1121,6 +1121,20 @@
qemuBlockJobProcessEventConcludedCopyAbort(virQEMUDriverPtr driver,
}
+static void
+qemuBlockJobProcessEventFailedActiveCommit(virDomainObjPtr vm,
+ qemuBlockJobDataPtr job)
+{
+VIR_DEBUG("active commit job '%s' on VM '%s' failed", job->name,
vm->def->name);
+
+if (!job->disk)
+return;
+
+virObjectUnref(job->disk->mirror);
+job->disk->mirror = NULL;
+}
+
+
static void
qemuBlockJobProcessEventConcludedCreate(virQEMUDriverPtr driver,
virDomainObjPtr vm,
@@ -1211,10 +1225,7 @@
qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr job,
break;
case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT:
-if (job->disk) {
-virObjectUnref(job->disk->mirror);
-job->disk->mirror = NULL;
-}
+qemuBlockJobProcessEventFailedActiveCommit(vm, job);
break;
case QEMU_BLOCKJOB_TYPE_CREATE:
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH for 5.7.0 2/3] qemu_blockjob: Print image path on failed security metadata move too
When a block job is completed, the security image metadata are
moved to the new image. If this fails an warning is printed, but
the message contains only domain name and lacks image paths. Put
them both into the warning message.
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_blockjob.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index c77a129bfc..80302fb139 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -646,8 +646,10 @@ qemuBlockJobEventProcessLegacyCompleted(virQEMUDriverPtr
driver,
virDomainLockImageDetach(driver->lockManager, vm, disk->src);
/* Move secret driver metadata */
-if (qemuSecurityMoveImageMetadata(driver, vm, disk->src, disk->mirror)
< 0)
-VIR_WARN("Unable to move disk metadata on vm %s", vm->def->name);
+if (qemuSecurityMoveImageMetadata(driver, vm, disk->src, disk->mirror)
< 0) {
+VIR_WARN("Unable to move disk metadata on vm %s from %s to %s",
+ vm->def->name, NULLSTR(disk->src->path),
NULLSTR(disk->mirror->path));
+}
virObjectUnref(disk->src);
disk->src = disk->mirror;
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH for 5.7.0 0/3] qemu_blockjob: Restore seclabels more frequently
At some points in blockjob code we were not restoring seclabels on backing chain qemu deceased to use. This leads to perms/XATTRs leak problem. Michal Prívozník (3): qemu_blockjob: Move active commit failed state handling into a function qemu_blockjob: Print image path on failed security metadata move too qemu_blockjob: Restore seclabels more frequently on job events src/qemu/qemu_blockjob.c | 46 ++-- 1 file changed, 40 insertions(+), 6 deletions(-) -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 1/3] adding unplug_timeout QEMU conf
On 8/30/19 8:15 AM, Christophe de Dinechin wrote: Daniel P. Berrangé writes: The reason for this timeout is that we originally promised something that we cannot deliver - a synchronous device detach API, while the operation itself is asynchronous. I'm not a fan of exposing it and making it configurable. I'm especially *not* a fan because the commit messages says this is a problem on certain architectures. Since we know what those arches are, we should use a larger timeout for those arches out of the box. Requiring admin to set a config param to fix the architectures is super unpleasant out of the box experiance. True, but also notice that 5 seconds is also already close to the attention span time limit for users [1]. So increasing it to 10s might bring people to believe things are stuck, unless you provide some sort of feedback that this is normal. https://www.nngroup.com/articles/response-times-3-important-limits/ Interesting link, thanks. About the user feedback due to long response delay: we're already breaking this with the setvcpus command, at least with PowerPC guests and a lot of vcpus being unplugged. Here's an example in which I am able to complete the command without kicking the timeout error (guest is idle, vcpu unplug is fast in this case): --- guest booted with 1 vcpu - added 39 extra vcpus. Operation takes a second, it's fast --- [danielhb@kop5 libvirt]$ sudo ./run tools/virsh setvcpus vcpus-test 40 --live [danielhb@kop5 libvirt]$ [danielhb@kop5 libvirt]$ --- removing them back [danielhb@kop5 libvirt]$ time sudo ./run tools/virsh setvcpus vcpus-test 1 --live real 0m21.695s user 0m0.119s sys 0m0.000s [danielhb@kop5 libvirt]$ This happens in PowerPC because the timeout is being considered not for the whole operation, but per device. Since I'm unplugging 39 devices and the 5 seconds timeout is refreshed for every operation, in theory the user can wait close to 39*5 seconds with the terminal frozen. Now, if we are to adhere to such UX standards (IMO, we should), I propose the following: - short term: increase PowerPC timeout to 10 seconds per device. Following the UX guideline above, this is the limit we can go without warning the user about the delay; - short term: for PowerPC guests, tune 'setvcpus' message to warn the user that the operation can take some time to complete; These 2 are simples changes and I can get it done for the next release without too much trouble. - mid/long term: I can look into the PowerPC guest implementation, see if there are device_del events being fired up in QMP and implement a better UX with more information about how the process is going. Something like "vcpu 1 out of 30 unplugged", "vcpu 2 out of 30 unplugged", or a progress bar, or whatever makes more sense to give the user a feeling of operation ongoing. Note that I'm suggesting PowerPC only changes due to what Daniel said earlier - we can't impact other users due to something that, at first glance, only PowerPC does different. I have a hunch that we should do for all archs, but I can't defend this claim without testing this in x86 at least. These short-term changes are easy to make it across the board though,so it's just a matter of removing "if PowerPC" in these changes. What do you think? Thanks, DHB 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 :| -- Cheers, Christophe de Dinechin (IRC c3d) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH for 5.7.0 1/3] qemu_blockjob: Move active commit failed state handling into a function
On Fri, Aug 30, 2019 at 15:19:06 +0200, Michal Privoznik wrote: > Currently, there are only a few lines of code so a separate > function was not necessary, but this will change. So instead of > putting all the new code under 'case > QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT' create a separate function. > Just like every other case has one. > > Signed-off-by: Michal Privoznik > --- > src/qemu/qemu_blockjob.c | 19 +++ > 1 file changed, 15 insertions(+), 4 deletions(-) ACK signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH for 5.7.0 2/3] qemu_blockjob: Print image path on failed security metadata move too
On Fri, Aug 30, 2019 at 15:19:07 +0200, Michal Privoznik wrote:
> When a block job is completed, the security image metadata are
> moved to the new image. If this fails an warning is printed, but
> the message contains only domain name and lacks image paths. Put
> them both into the warning message.
>
> Signed-off-by: Michal Privoznik
> ---
> src/qemu/qemu_blockjob.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
ACK
>
> diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
> index c77a129bfc..80302fb139 100644
> --- a/src/qemu/qemu_blockjob.c
> +++ b/src/qemu/qemu_blockjob.c
> @@ -646,8 +646,10 @@ qemuBlockJobEventProcessLegacyCompleted(virQEMUDriverPtr
> driver,
> virDomainLockImageDetach(driver->lockManager, vm, disk->src);
>
> /* Move secret driver metadata */
> -if (qemuSecurityMoveImageMetadata(driver, vm, disk->src,
> disk->mirror) < 0)
> -VIR_WARN("Unable to move disk metadata on vm %s", vm->def->name);
> +if (qemuSecurityMoveImageMetadata(driver, vm, disk->src,
> disk->mirror) < 0) {
> +VIR_WARN("Unable to move disk metadata on vm %s from %s to %s",
> + vm->def->name, NULLSTR(disk->src->path),
> NULLSTR(disk->mirror->path));
perhaps also log disk->dst
> +}
>
> virObjectUnref(disk->src);
> disk->src = disk->mirror;
> --
> 2.21.0
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH for 5.7.0 3/3] qemu_blockjob: Restore seclabels more frequently on job events
On Fri, Aug 30, 2019 at 15:19:08 +0200, Michal Privoznik wrote:
> If a block job reaches failed/cancelled state, or is completed
> without pivot then qemu no longer uses the mirror image. Since
> we've set its seclabels we must restore them back to avoid
> leaking perms/XATTRs.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1741456
>
> Signed-off-by: Michal Privoznik
> ---
> src/qemu/qemu_blockjob.c | 25 +++--
> 1 file changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
> index 80302fb139..8411d8e223 100644
> --- a/src/qemu/qemu_blockjob.c
> +++ b/src/qemu/qemu_blockjob.c
> @@ -656,6 +656,13 @@ qemuBlockJobEventProcessLegacyCompleted(virQEMUDriverPtr
> driver,
> } else {
> if (disk->mirror) {
> virDomainLockImageDetach(driver->lockManager, vm, disk->mirror);
> +
> +/* QEMU no longer uses the image, so we can restore its label. */
> +if (qemuSecurityRestoreImageLabel(driver, vm, disk->mirror,
> true) < 0) {
> +VIR_WARN("Unable to restore security labels on vm %s disk
> %s",
> + vm->def->name, NULLSTR(disk->mirror->path));
> +}
> +
So unfortunately this is true only for the block copy operation. Active
block commit will still continue using the image in read-only mode.
Also I'm afraid I remember why this code does not have any security
labeling code. The mirror may in fact contain parts of the original
backing chain (if the --reuse-external flag was used) and in that case
we'd not be able to tell which ones we want to remove labelling from, so
the only "safe" operation is to remove labels/locks from the top image
only.
> virObjectUnref(disk->mirror);
> }
> }
> @@ -725,6 +732,13 @@ qemuBlockJobEventProcessLegacy(virQEMUDriverPtr driver,
> case VIR_DOMAIN_BLOCK_JOB_CANCELED:
> if (disk->mirror) {
> virDomainLockImageDetach(driver->lockManager, vm, disk->mirror);
> +
> +/* QEMU no longer uses the image, so we can restore its label. */
> +if (qemuSecurityRestoreImageLabel(driver, vm, disk->mirror,
> true) < 0) {
> +VIR_WARN("Unable to restore security labels on vm %s disk
> %s",
> + vm->def->name, NULLSTR(disk->mirror->path));
> +}
> +
> virObjectUnref(disk->mirror);
> disk->mirror = NULL;
> }
> @@ -1124,7 +1138,8 @@
> qemuBlockJobProcessEventConcludedCopyAbort(virQEMUDriverPtr driver,
>
>
> static void
> -qemuBlockJobProcessEventFailedActiveCommit(virDomainObjPtr vm,
> +qemuBlockJobProcessEventFailedActiveCommit(virQEMUDriverPtr driver,
> + virDomainObjPtr vm,
> qemuBlockJobDataPtr job)
> {
> VIR_DEBUG("active commit job '%s' on VM '%s' failed", job->name,
> vm->def->name);
> @@ -1132,6 +1147,12 @@
> qemuBlockJobProcessEventFailedActiveCommit(virDomainObjPtr vm,
> if (!job->disk)
> return;
>
> +/* QEMU no longer uses the image, so we can restore its label. */
> +if (qemuSecurityRestoreImageLabel(driver, vm, job->disk->mirror, true) <
> 0) {
> +VIR_WARN("Unable to restore security labels on vm %s disk %s",
> + vm->def->name, NULLSTR(job->disk->mirror->path));
So, here this must return the security labels to readonly state rather
than removing it completely as this is still used.
I'm sorry these points didn't occur to me when we were discussing this
code :/
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] vircgroupv2: fix abort in VIR_AUTOFREE
Introduced by commit that
tried to fix an issue where we would fail to parse values from files.
We cannot change the original pointer that is going to be used by
VIR_AUTOFREE.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1747440
Signed-off-by: Pavel Hrdina
---
Sigh, shame on me!
src/util/vircgroupv2.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/util/vircgroupv2.c b/src/util/vircgroupv2.c
index c62ee0d933..2aca4e5d62 100644
--- a/src/util/vircgroupv2.c
+++ b/src/util/vircgroupv2.c
@@ -849,6 +849,7 @@ virCgroupV2GetBlkioDeviceWeight(virCgroupPtr group,
VIR_AUTOFREE(char *) path = NULL;
VIR_AUTOFREE(char *) str = NULL;
VIR_AUTOFREE(char *) value = NULL;
+char *tmp;
if (virCgroupV2PathOfController(group, VIR_CGROUP_CONTROLLER_BLKIO,
"io.weight", &path) < 0) {
@@ -869,7 +870,7 @@ virCgroupV2GetBlkioDeviceWeight(virCgroupPtr group,
if (!str) {
*weight = 0;
-} else if (virStrToLong_ui(str, &str, 10, weight) < 0) {
+} else if (virStrToLong_ui(str, &tmp, 10, weight) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unable to parse '%s' as an integer"),
str);
@@ -1576,6 +1577,7 @@ virCgroupV2GetCpuCfsQuota(virCgroupPtr group,
long long *cfs_quota)
{
VIR_AUTOFREE(char *) str = NULL;
+char *tmp;
if (virCgroupGetValueStr(group, VIR_CGROUP_CONTROLLER_CPU,
"cpu.max", &str) < 0) {
@@ -1587,7 +1589,7 @@ virCgroupV2GetCpuCfsQuota(virCgroupPtr group,
return 0;
}
-if (virStrToLong_ll(str, &str, 10, cfs_quota) < 0) {
+if (virStrToLong_ll(str, &tmp, 10, cfs_quota) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Failed to parse value '%s' from cpu.max."), str);
return -1;
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vircgroupv2: fix abort in VIR_AUTOFREE
On Fri, Aug 30, 2019 at 16:26:23 +0200, Pavel Hrdina wrote: > Introduced by commit that > tried to fix an issue where we would fail to parse values from files. > > We cannot change the original pointer that is going to be used by > VIR_AUTOFREE. > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1747440 > > Signed-off-by: Pavel Hrdina > --- > > Sigh, shame on me! Lol, ACK signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH for 5.7.0 3/3] qemu_blockjob: Restore seclabels more frequently on job events
On 8/30/19 3:42 PM, Peter Krempa wrote:
On Fri, Aug 30, 2019 at 15:19:08 +0200, Michal Privoznik wrote:
If a block job reaches failed/cancelled state, or is completed
without pivot then qemu no longer uses the mirror image. Since
we've set its seclabels we must restore them back to avoid
leaking perms/XATTRs.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1741456
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_blockjob.c | 25 +++--
1 file changed, 23 insertions(+), 2 deletions(-)
@@ -1124,7 +1138,8 @@
qemuBlockJobProcessEventConcludedCopyAbort(virQEMUDriverPtr driver,
static void
-qemuBlockJobProcessEventFailedActiveCommit(virDomainObjPtr vm,
+qemuBlockJobProcessEventFailedActiveCommit(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
qemuBlockJobDataPtr job)
{
VIR_DEBUG("active commit job '%s' on VM '%s' failed", job->name,
vm->def->name);
@@ -1132,6 +1147,12 @@
qemuBlockJobProcessEventFailedActiveCommit(virDomainObjPtr vm,
if (!job->disk)
return;
+/* QEMU no longer uses the image, so we can restore its label. */
+if (qemuSecurityRestoreImageLabel(driver, vm, job->disk->mirror, true) <
0) {
+VIR_WARN("Unable to restore security labels on vm %s disk %s",
+ vm->def->name, NULLSTR(job->disk->mirror->path));
So, here this must return the security labels to readonly state rather
than removing it completely as this is still used.
That is not implemented :-( Our virSecuritySELinuxRestoreImageLabel()
acts like 'restorecon $path' which is not what we need here. You know
what? I'll remove the XATTRs in all three cases (at least for the time
being) as it's safe to do so (the worst thing is that we won't restore
the original label - but we weren't doing that anyway). For the first
two (legacy mode) we can't know if somebody else is not using the
backing chain, and in this blockdev case we could know that but that
would require bigger fix.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] qemu: monitor: Fix formatting of 'offset' in qemuMonitorJSONSaveMemory
The offset is unsigned long long thus 'U' must be used. Signed-off-by: Peter Krempa --- src/qemu/qemu_monitor_json.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index da1e89dded..e4404f0199 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -3123,7 +3123,7 @@ static int qemuMonitorJSONSaveMemory(qemuMonitorPtr mon, int ret = -1; virJSONValuePtr cmd = qemuMonitorJSONMakeCommand(cmdtype, "U:val", offset, - "u:size", length, + "U:size", length, "s:filename", path, NULL); virJSONValuePtr reply = NULL; -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] qemu: Fix formatting of (some) unsigned long long values to JSON (blockdev-add saga)
Currently, libvirt would regress to computer-middle-ages by not being able to use more than 4GiB images for snapshots without corrupting data. Fix it by using the proper formatting modifier. Peter Krempa (3): qemu: block: Use correct type when creating image size JSON entries tests: qemublock: Use bigger numbers as dummy capacity/physical qemu: monitor: Fix formatting of 'offset' in qemuMonitorJSONSaveMemory src/qemu/qemu_block.c| 12 ++-- src/qemu/qemu_monitor_json.c | 2 +- tests/qemublocktest.c| 4 ++-- .../qemublocktestdata/imagecreate/luks-encopts.json | 4 ++-- tests/qemublocktestdata/imagecreate/luks-noopts.json | 4 ++-- .../imagecreate/network-gluster-qcow2.json | 4 ++-- .../imagecreate/network-rbd-qcow2.json | 4 ++-- .../imagecreate/network-sheepdog-qcow2.json | 4 ++-- .../imagecreate/network-ssh-qcow2.json | 4 ++-- .../imagecreate/qcow2-backing-luks.json | 4 ++-- .../imagecreate/qcow2-backing-raw-nbd.json | 4 ++-- .../imagecreate/qcow2-backing-raw.json | 4 ++-- .../imagecreate/qcow2-luks-encopts-backing.json | 4 ++-- .../imagecreate/qcow2-luks-encopts.json | 4 ++-- .../imagecreate/qcow2-luks-noopts.json | 4 ++-- tests/qemublocktestdata/imagecreate/qcow2.json | 4 ++-- tests/qemublocktestdata/imagecreate/raw.json | 2 +- 17 files changed, 36 insertions(+), 36 deletions(-) -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] qemu: block: Use correct type when creating image size JSON entries
The 'u' modifier creates a unsigned int JSON attribute but the disk size and capacity fields are unsigned long long. If the size of the created image would be more than 4GiB we'd overflow and create sub-4G image. Signed-off-by: Peter Krempa --- src/qemu/qemu_block.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 66b1d116d8..e33aad4458 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -2034,7 +2034,7 @@ qemuBlockStorageSourceCreateGetFormatPropsGeneric(virStorageSourcePtr src, if (virJSONValueObjectCreate(&props, "s:driver", driver, "s:file", src->nodestorage, - "u:size", src->capacity, + "U:size", src->capacity, NULL) < 0) return -1; @@ -2100,7 +2100,7 @@ qemuBlockStorageSourceCreateGetFormatPropsLUKS(virStorageSourcePtr src, if (virJSONValueObjectAdd(luksprops, "s:driver", "luks", "s:file", src->nodestorage, - "u:size", src->capacity, + "U:size", src->capacity, NULL) < 0) return -1; @@ -2153,7 +2153,7 @@ qemuBlockStorageSourceCreateGetFormatPropsQcow2(virStorageSourcePtr src, if (virJSONValueObjectCreate(&qcow2props, "s:driver", "qcow2", "s:file", src->nodestorage, - "u:size", src->capacity, + "U:size", src->capacity, "S:version", qcow2version, NULL) < 0) return -1; @@ -2177,7 +2177,7 @@ qemuBlockStorageSourceCreateGetFormatPropsQcow(virStorageSourcePtr src, if (virJSONValueObjectCreate(&qcowprops, "s:driver", "qcow", "s:file", src->nodestorage, - "u:size", src->capacity, + "U:size", src->capacity, NULL) < 0) return -1; @@ -2200,7 +2200,7 @@ qemuBlockStorageSourceCreateGetFormatPropsQed(virStorageSourcePtr src, if (virJSONValueObjectCreate(&qedprops, "s:driver", "qed", "s:file", src->nodestorage, - "u:size", src->capacity, + "U:size", src->capacity, NULL) < 0) return -1; @@ -2373,7 +2373,7 @@ qemuBlockStorageSourceCreateGetStorageProps(virStorageSourcePtr src, "s:driver", driver, "S:filename", filename, "A:location", &location, - "u:size", src->physical, + "U:size", src->physical, NULL) < 0) return -1; -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] tests: qemublock: Use bigger numbers as dummy capacity/physical
Actually test that the full range is available.
Signed-off-by: Peter Krempa
---
tests/qemublocktest.c | 4 ++--
tests/qemublocktestdata/imagecreate/luks-encopts.json | 4 ++--
tests/qemublocktestdata/imagecreate/luks-noopts.json | 4 ++--
.../qemublocktestdata/imagecreate/network-gluster-qcow2.json | 4 ++--
tests/qemublocktestdata/imagecreate/network-rbd-qcow2.json| 4 ++--
.../qemublocktestdata/imagecreate/network-sheepdog-qcow2.json | 4 ++--
tests/qemublocktestdata/imagecreate/network-ssh-qcow2.json| 4 ++--
tests/qemublocktestdata/imagecreate/qcow2-backing-luks.json | 4 ++--
.../qemublocktestdata/imagecreate/qcow2-backing-raw-nbd.json | 4 ++--
tests/qemublocktestdata/imagecreate/qcow2-backing-raw.json| 4 ++--
.../imagecreate/qcow2-luks-encopts-backing.json | 4 ++--
tests/qemublocktestdata/imagecreate/qcow2-luks-encopts.json | 4 ++--
tests/qemublocktestdata/imagecreate/qcow2-luks-noopts.json| 4 ++--
tests/qemublocktestdata/imagecreate/qcow2.json| 4 ++--
tests/qemublocktestdata/imagecreate/raw.json | 2 +-
15 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c
index 0bc4b65449..1bf72a4615 100644
--- a/tests/qemublocktest.c
+++ b/tests/qemublocktest.c
@@ -420,8 +420,8 @@ testQemuImageCreate(const void *opaque)
return -1;
/* fake some sizes */
-src->capacity = 1337;
-src->physical = 42;
+src->capacity = UINT_MAX * 2ULL;
+src->physical = UINT_MAX + 1ULL;
if (qemuDomainValidateStorageSource(src, data->qemuCaps) < 0)
return -1;
diff --git a/tests/qemublocktestdata/imagecreate/luks-encopts.json
b/tests/qemublocktestdata/imagecreate/luks-encopts.json
index f065ad89a7..c5ca863f5b 100644
--- a/tests/qemublocktestdata/imagecreate/luks-encopts.json
+++ b/tests/qemublocktestdata/imagecreate/luks-encopts.json
@@ -2,7 +2,7 @@ protocol:
{
"driver": "file",
"filename": "/var/lib/libvirt/images/i.img",
- "size": 42
+ "size": 4294967296
}
format:
@@ -15,5 +15,5 @@ format:
"ivgen-hash-alg": "sha256",
"driver": "luks",
"file": "0123456789ABCDEF0123456789ABCDE",
- "size": 1337
+ "size": 8589934590
}
diff --git a/tests/qemublocktestdata/imagecreate/luks-noopts.json
b/tests/qemublocktestdata/imagecreate/luks-noopts.json
index 1ea1948119..8a0944151d 100644
--- a/tests/qemublocktestdata/imagecreate/luks-noopts.json
+++ b/tests/qemublocktestdata/imagecreate/luks-noopts.json
@@ -2,7 +2,7 @@ protocol:
{
"driver": "file",
"filename": "/var/lib/libvirt/images/i.img",
- "size": 42
+ "size": 4294967296
}
format:
@@ -10,5 +10,5 @@ format:
"key-secret": "0123456789ABCDEF0123456789ABCDE-encalias",
"driver": "luks",
"file": "0123456789ABCDEF0123456789ABCDE",
- "size": 1337
+ "size": 8589934590
}
diff --git a/tests/qemublocktestdata/imagecreate/network-gluster-qcow2.json
b/tests/qemublocktestdata/imagecreate/network-gluster-qcow2.json
index aee7bfd401..3e35beb088 100644
--- a/tests/qemublocktestdata/imagecreate/network-gluster-qcow2.json
+++ b/tests/qemublocktestdata/imagecreate/network-gluster-qcow2.json
@@ -17,12 +17,12 @@ protocol:
}
]
},
- "size": 42
+ "size": 4294967296
}
format:
{
"driver": "qcow2",
"file": "0123456789ABCDEF0123456789ABCDE",
- "size": 1337
+ "size": 8589934590
}
diff --git a/tests/qemublocktestdata/imagecreate/network-rbd-qcow2.json
b/tests/qemublocktestdata/imagecreate/network-rbd-qcow2.json
index 56d9c0f1ed..67e2679dae 100644
--- a/tests/qemublocktestdata/imagecreate/network-rbd-qcow2.json
+++ b/tests/qemublocktestdata/imagecreate/network-rbd-qcow2.json
@@ -15,12 +15,12 @@ protocol:
}
]
},
- "size": 42
+ "size": 4294967296
}
format:
{
"driver": "qcow2",
"file": "0123456789ABCDEF0123456789ABCDE",
- "size": 1337
+ "size": 8589934590
}
diff --git a/tests/qemublocktestdata/imagecreate/network-sheepdog-qcow2.json
b/tests/qemublocktestdata/imagecreate/network-sheepdog-qcow2.json
index b7272625a2..d86bef6bc8 100644
--- a/tests/qemublocktestdata/imagecreate/network-sheepdog-qcow2.json
+++ b/tests/qemublocktestdata/imagecreate/network-sheepdog-qcow2.json
@@ -9,12 +9,12 @@ protocol:
},
"vdi": "asdf/i.qcow2"
},
- "size": 42
+ "size": 4294967296
}
format:
{
"driver": "qcow2",
"file": "0123456789ABCDEF0123456789ABCDE",
- "size": 1337
+ "size": 8589934590
}
diff --git a/tests/qemublocktestdata/imagecreate/network-ssh-qcow2.json
b/tests/qemublocktestdata/imagecreate/network-ssh-qcow2.json
index 31416ed4fc..d58054c081 100644
--- a/tests/qemublocktestdata/imagecreate/network-ssh-qcow2.json
+++ b/tests/qemublocktestdata/imagecreate/network-ssh-qcow2.json
@@ -8,12 +8,12 @@ protocol:
"port": "1234"
}
},
- "size": 42
+ "size": 4294967296
}
format:
{
"driver": "qcow2",
"file": "0123456789ABCDEF0123456789ABCDE",
- "size": 1337
+ "size"
[libvirt] [PATCH for 5.7.0 v2 0/3] qemu_blockjob: Remove secdriver metadata more frequently
v2 of: https://www.redhat.com/archives/libvir-list/2019-August/msg01418.html Patches 1/3 and 2/3 were ACKed but I did not pushed them yet - I'd like to push these all at once. diff to v1: - Instead of restoring seclabels on the backing chain, remove the metadata as we can't know if somebody else is not using some parts of the chain. Michal Prívozník (3): qemu_blockjob: Move active commit failed state handling into a function qemu_blockjob: Print image path on failed security metadata move too qemu_blockjob: Remove secdriver metadata more frequently src/qemu/qemu_blockjob.c | 78 1 file changed, 72 insertions(+), 6 deletions(-) -- 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH for 5.7.0 v2 2/3] qemu_blockjob: Print image path on failed security metadata move too
When a block job is completed, the security image metadata are
moved to the new image. If this fails an warning is printed, but
the message contains only domain name and lacks image paths. Put
them both into the warning message.
Signed-off-by: Michal Privoznik
ACKed-by: Peter Krempa
---
src/qemu/qemu_blockjob.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index c77a129bfc..1b22689e0c 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -646,8 +646,14 @@ qemuBlockJobEventProcessLegacyCompleted(virQEMUDriverPtr
driver,
virDomainLockImageDetach(driver->lockManager, vm, disk->src);
/* Move secret driver metadata */
-if (qemuSecurityMoveImageMetadata(driver, vm, disk->src, disk->mirror)
< 0)
-VIR_WARN("Unable to move disk metadata on vm %s", vm->def->name);
+if (qemuSecurityMoveImageMetadata(driver, vm, disk->src, disk->mirror)
< 0) {
+VIR_WARN("Unable to move disk metadata on "
+ "vm %s from %s to %s (disk target %s)",
+ vm->def->name,
+ NULLSTR(disk->src->path),
+ NULLSTR(disk->mirror->path),
+ disk->dst);
+}
virObjectUnref(disk->src);
disk->src = disk->mirror;
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH for 5.7.0 v2 1/3] qemu_blockjob: Move active commit failed state handling into a function
Currently, there are only a few lines of code so a separate
function was not necessary, but this will change. So instead of
putting all the new code under 'case
QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT' create a separate function.
Just like every other case has one.
Signed-off-by: Michal Privoznik
ACKed-by: Peter Krempa
---
src/qemu/qemu_blockjob.c | 19 +++
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 3003e9c518..c77a129bfc 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -1121,6 +1121,20 @@
qemuBlockJobProcessEventConcludedCopyAbort(virQEMUDriverPtr driver,
}
+static void
+qemuBlockJobProcessEventFailedActiveCommit(virDomainObjPtr vm,
+ qemuBlockJobDataPtr job)
+{
+VIR_DEBUG("active commit job '%s' on VM '%s' failed", job->name,
vm->def->name);
+
+if (!job->disk)
+return;
+
+virObjectUnref(job->disk->mirror);
+job->disk->mirror = NULL;
+}
+
+
static void
qemuBlockJobProcessEventConcludedCreate(virQEMUDriverPtr driver,
virDomainObjPtr vm,
@@ -1211,10 +1225,7 @@
qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr job,
break;
case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT:
-if (job->disk) {
-virObjectUnref(job->disk->mirror);
-job->disk->mirror = NULL;
-}
+qemuBlockJobProcessEventFailedActiveCommit(vm, job);
break;
case QEMU_BLOCKJOB_TYPE_CREATE:
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH for 5.7.0 v2 3/3] qemu_blockjob: Remove secdriver metadata more frequently
If a block job reaches failed/cancelled state, or is completed
without pivot then we must remove security driver metadata
associated to the backing chain so that we don't leave any
metadata behind.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1741456
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_blockjob.c | 59
1 file changed, 54 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 1b22689e0c..a991309ee7 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -659,7 +659,23 @@ qemuBlockJobEventProcessLegacyCompleted(virQEMUDriverPtr
driver,
disk->src = disk->mirror;
} else {
if (disk->mirror) {
+virStorageSourcePtr n;
+
virDomainLockImageDetach(driver->lockManager, vm, disk->mirror);
+
+/* Ideally, we would restore seclabels on the backing chain here
+ * but we don't know if somebody else is not using parts of it.
+ * Remove security driver metadata so that they are not leaked. */
+for (n = disk->mirror; virStorageSourceIsBacking(n); n =
n->backingStore) {
+if (qemuSecurityMoveImageMetadata(driver, vm, n, NULL) < 0) {
+VIR_WARN("Unable to remove disk metadata on "
+ "vm %s from %s (disk target %s)",
+ vm->def->name,
+ NULLSTR(disk->src->path),
+ disk->dst);
+}
+}
+
virObjectUnref(disk->mirror);
}
}
@@ -728,7 +744,23 @@ qemuBlockJobEventProcessLegacy(virQEMUDriverPtr driver,
case VIR_DOMAIN_BLOCK_JOB_FAILED:
case VIR_DOMAIN_BLOCK_JOB_CANCELED:
if (disk->mirror) {
+virStorageSourcePtr n;
+
virDomainLockImageDetach(driver->lockManager, vm, disk->mirror);
+
+/* Ideally, we would restore seclabels on the backing chain here
+ * but we don't know if somebody else is not using parts of it.
+ * Remove security driver metadata so that they are not leaked. */
+for (n = disk->mirror; virStorageSourceIsBacking(n); n =
n->backingStore) {
+if (qemuSecurityMoveImageMetadata(driver, vm, n, NULL) < 0) {
+VIR_WARN("Unable to remove disk metadata on "
+ "vm %s from %s (disk target %s)",
+ vm->def->name,
+ NULLSTR(disk->src->path),
+ disk->dst);
+}
+}
+
virObjectUnref(disk->mirror);
disk->mirror = NULL;
}
@@ -1128,16 +1160,33 @@
qemuBlockJobProcessEventConcludedCopyAbort(virQEMUDriverPtr driver,
static void
-qemuBlockJobProcessEventFailedActiveCommit(virDomainObjPtr vm,
+qemuBlockJobProcessEventFailedActiveCommit(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
qemuBlockJobDataPtr job)
{
+virDomainDiskDefPtr disk = job->disk;
+virStorageSourcePtr n;
+
VIR_DEBUG("active commit job '%s' on VM '%s' failed", job->name,
vm->def->name);
-if (!job->disk)
+if (!disk)
return;
-virObjectUnref(job->disk->mirror);
-job->disk->mirror = NULL;
+/* Ideally, we would make the backing chain read only again (yes, SELinux
+ * can do that using different labels). But that is not implemented yet and
+ * not leaking security driver metadata is more important. */
+for (n = disk->mirror; virStorageSourceIsBacking(n); n = n->backingStore) {
+if (qemuSecurityMoveImageMetadata(driver, vm, n, NULL) < 0) {
+VIR_WARN("Unable to remove disk metadata on "
+ "vm %s from %s (disk target %s)",
+ vm->def->name,
+ NULLSTR(disk->src->path),
+ disk->dst);
+}
+}
+
+virObjectUnref(disk->mirror);
+disk->mirror = NULL;
}
@@ -1231,7 +1280,7 @@
qemuBlockJobEventProcessConcludedTransition(qemuBlockJobDataPtr job,
break;
case QEMU_BLOCKJOB_TYPE_ACTIVE_COMMIT:
-qemuBlockJobProcessEventFailedActiveCommit(vm, job);
+qemuBlockJobProcessEventFailedActiveCommit(driver, vm, job);
break;
case QEMU_BLOCKJOB_TYPE_CREATE:
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] tests: qemublock: Use bigger numbers as dummy capacity/physical
On Fri, Aug 30, 2019 at 04:45:04PM +0200, Peter Krempa wrote: Actually test that the full range is available. Signed-off-by: Peter Krempa --- tests/qemublocktest.c | 4 ++-- tests/qemublocktestdata/imagecreate/luks-encopts.json | 4 ++-- tests/qemublocktestdata/imagecreate/luks-noopts.json | 4 ++-- .../qemublocktestdata/imagecreate/network-gluster-qcow2.json | 4 ++-- tests/qemublocktestdata/imagecreate/network-rbd-qcow2.json| 4 ++-- .../qemublocktestdata/imagecreate/network-sheepdog-qcow2.json | 4 ++-- tests/qemublocktestdata/imagecreate/network-ssh-qcow2.json| 4 ++-- tests/qemublocktestdata/imagecreate/qcow2-backing-luks.json | 4 ++-- .../qemublocktestdata/imagecreate/qcow2-backing-raw-nbd.json | 4 ++-- tests/qemublocktestdata/imagecreate/qcow2-backing-raw.json| 4 ++-- .../imagecreate/qcow2-luks-encopts-backing.json | 4 ++-- tests/qemublocktestdata/imagecreate/qcow2-luks-encopts.json | 4 ++-- tests/qemublocktestdata/imagecreate/qcow2-luks-noopts.json| 4 ++-- tests/qemublocktestdata/imagecreate/qcow2.json| 4 ++-- tests/qemublocktestdata/imagecreate/raw.json | 2 +- 15 files changed, 29 insertions(+), 29 deletions(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] qemu: monitor: Fix formatting of 'offset' in qemuMonitorJSONSaveMemory
On Fri, Aug 30, 2019 at 04:45:05PM +0200, Peter Krempa wrote: The offset is unsigned long long thus 'U' must be used. Signed-off-by: Peter Krempa --- src/qemu/qemu_monitor_json.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] qemu: block: Use correct type when creating image size JSON entries
On Fri, Aug 30, 2019 at 04:45:03PM +0200, Peter Krempa wrote: The 'u' modifier creates a unsigned int JSON attribute but the disk size an unsigned and capacity fields are unsigned long long. If the size of the created image would be more than 4GiB we'd overflow and create sub-4G image. Signed-off-by: Peter Krempa --- src/qemu/qemu_block.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) Depending on how broken things get, qemuBlockStorageSourceGetGlusterProps might need a similar change ;) Reviewed-by: Ján Tomko Jano signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] On present using dummy hostdev usb device
On 8/30/19 2:30 PM, Roman Kagan wrote: On Fri, Aug 30, 2019 at 10:09:06AM +0100, Daniel P. Berrangé wrote: On Fri, Aug 30, 2019 at 08:44:03AM +, Nikolay Shirokovskiy wrote: Hi, all! We use an interesting approach when starting/migrating/etc domain with usb hostdev with startupPolicy=optional. We add qemu usb-host device with missing hostaddr/hostbus parameters (dummy device). I guess there are 2 reasons why we do it. First without dummy device migration will fail as described in [1]. Second is an interesting property of dummy device that qemu starts to monitor for attaching of usb devices and binds the first attached to node to the dummy device. So one can start a domain with missing hostdev and attach it later or migrate a domain then detach hostdev on source and attach it on destination. But as qemu binds the first attached device this is not reliable, to say the least. And after all this does not work if domain uses distinct mount namespace which is default. Even without mount namespaces, it should fail as QEMU is running non-root and libvirt won't have granted access to any host USB devices in /dev, and also SELinux policy will forbid this. Right, but the case with mount namespaces is particularly problematic: if the device open fails due to missing device node, libusb removes the device from its internal device list. This results in the following scenario: - libvirt adds a dummy usb-host device to QEMU in place of a missing device - QEMU (via libusb) installs a watch for udev add events - the physical device is plugged into the host - QEMU detects the addition of the device and, since the dummy device matches everything, tries to open it - by this time libvirt may have not created a device node in QEMU's mount namespace, so the open fails due to missing device node, and libusb removes the device from its internal list - libvirt removes the dummy usb-host device and adds the actual usb-host device - QEMU fails to open it because it's no longer seen by libusb There is a bug filed against libusb exactly for this: https://bugzilla.redhat.com/show_bug.cgi?id=1595525 BTW: you don't have to migrate, it's sufficient to start a domain with a missing USB and startupPolicy='optional' and then physically plug it into the host and then try to hotplug it into the domain. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH for 5.7.0 v2 3/3] qemu_blockjob: Remove secdriver metadata more frequently
On Fri, Aug 30, 2019 at 16:56:10 +0200, Michal Privoznik wrote: > If a block job reaches failed/cancelled state, or is completed > without pivot then we must remove security driver metadata > associated to the backing chain so that we don't leave any > metadata behind. > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1741456 > > Signed-off-by: Michal Privoznik > --- > src/qemu/qemu_blockjob.c | 59 > 1 file changed, 54 insertions(+), 5 deletions(-) ACK signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] On present using dummy hostdev usb device
On Fri, Aug 30, 2019 at 05:08:52PM +0200, Michal Privoznik wrote: > On 8/30/19 2:30 PM, Roman Kagan wrote: > > On Fri, Aug 30, 2019 at 10:09:06AM +0100, Daniel P. Berrangé wrote: > > > On Fri, Aug 30, 2019 at 08:44:03AM +, Nikolay Shirokovskiy wrote: > > > > Hi, all! > > > > > > > >We use an interesting approach when starting/migrating/etc domain > > > > with usb > > > > hostdev with startupPolicy=optional. We add qemu usb-host device with > > > > missing hostaddr/hostbus parameters (dummy device). I guess there are > > > > 2 reasons why we do it. First without dummy device migration will fail > > > > as > > > > described in [1]. Second is an interesting property of dummy device that > > > > qemu starts to monitor for attaching of usb devices and binds the first > > > > attached to node to the dummy device. So one can start a domain with > > > > missing hostdev and attach it later or migrate a domain then detach > > > > hostdev on source and attach it on destination. But as qemu binds the > > > > first attached device this is not reliable, to say the least. And after > > > > all this does not work if domain uses distinct mount namespace which > > > > is default. > > > > > > Even without mount namespaces, it should fail as QEMU is running non-root > > > and libvirt won't have granted access to any host USB devices in /dev, and > > > also SELinux policy will forbid this. > > > > Right, but the case with mount namespaces is particularly problematic: > > if the device open fails due to missing device node, libusb removes the > > device from its internal device list. This results in the following > > scenario: > > > > - libvirt adds a dummy usb-host device to QEMU in place of a missing > >device > > > > - QEMU (via libusb) installs a watch for udev add events > > > > - the physical device is plugged into the host > > > > - QEMU detects the addition of the device and, since the dummy device > >matches everything, tries to open it > > > > - by this time libvirt may have not created a device node in QEMU's > >mount namespace, so the open fails due to missing device node, and > >libusb removes the device from its internal list > > > > - libvirt removes the dummy usb-host device and adds the actual usb-host > >device > > > > - QEMU fails to open it because it's no longer seen by libusb > > There is a bug filed against libusb exactly for this: > > https://bugzilla.redhat.com/show_bug.cgi?id=1595525 > > BTW: you don't have to migrate, it's sufficient to start a domain with a > missing USB and startupPolicy='optional' and then physically plug it into > the host and then try to hotplug it into the domain. AFAIR, the startupPolicy=optional was never really intended to allow for the device to be dynamically added after startup. It was only trying drop the device if it didn't exist at startup. If we do want a way to dynamically add after startup, then libvirt itself would have to monitor the USB devices on the host, and then perform QMP commands to hot-add to QEMU. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] tests: qemublock: Use bigger numbers as dummy capacity/physical
Peter Krempa writes:
> Actually test that the full range is available.
>
[...]
> return -1;
> diff --git a/tests/qemublocktestdata/imagecreate/luks-encopts.json
> b/tests/qemublocktestdata/imagecreate/luks-encopts.json
> index f065ad89a7..c5ca863f5b 100644
> --- a/tests/qemublocktestdata/imagecreate/luks-encopts.json
> +++ b/tests/qemublocktestdata/imagecreate/luks-encopts.json
> @@ -2,7 +2,7 @@ protocol:
> {
>"driver": "file",
>"filename": "/var/lib/libvirt/images/i.img",
> - "size": 42
> + "size": 4294967296
> }
Does any of these tests actually require that much space to run?
If so, what is the impact on QE teams?
--
Cheers,
Christophe de Dinechin (IRC c3d)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Fwd: libvirtd failing on MacOS in setgroups
On Thu, 22 Aug 2019 at 05:34, Daniel P. Berrangé wrote:
>
> On Wed, Aug 21, 2019 at 09:34:05PM +0300, Roman Bolshakov wrote:
> > On Wed, Aug 21, 2019 at 05:55:51PM +0100, Daniel P. Berrangé wrote:
> > > On Wed, Aug 21, 2019 at 12:47:03PM -0400, Marcus Furlong wrote:
> > > > On Wed, 21 Aug 2019 at 08:23, Daniel P. Berrangé
> > > > wrote:
> > > > >
> > > > > On Tue, Aug 20, 2019 at 11:11:07AM -0400, Marcus Furlong wrote:
> > > > > > Resend to libvir-list in case that is more appropriate:
> > > > > >
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > I get the following error when running libvirtd on MacOS as root:
> > > > > >
> > > > > > 2019-07-11 00:12:33.673+: 123145573953536: error :
> > > > > > qemuProcessQMPLaunch:8501 : internal error: Failed to start QEMU
> > > > > > binary /usr/local/bin/qemu-system-x86_64 for probing: libvirt:
> > > > > > error
> > > > > > : cannot set supplemental groups: Invalid argument
> > > > >
> > > > > Are you able to run 'strace' (or whatever MacOS eqiuv is) to see
> > > > > the values passed to setgroups when it fails ?
> > > >
> > > > I ran `dtruss -f -l -s /usr/local/sbin/libvirtd` but the setgroups
> > > > calls seem to be missing.
> > > >
> > > > Looking at other sources, it seems like some have special treatment of
> > > > setgroups on MacOS, e.g. samba:
> > > >
> > > >
> > > > https://github.com/samba-team/samba/blob/v4-11-stable/source3/smbd/sec_ctx.c#L261-#L305
> > > >
> > > > Perhaps this is needed for libvirt?
> > >
> > > The capping of ngroups to NGROUPS_MAX looks like a possibe reason.
> > >
> > > Adding this debug might show us if we're exceeding it:
> > >
> > > diff --git a/src/util/virutil.c b/src/util/virutil.c
> > > index 89d2cf011f..effc02b898 100644
> > > --- a/src/util/virutil.c
> > > +++ b/src/util/virutil.c
> > > @@ -1043,6 +1043,7 @@ virSetUIDGID(uid_t uid, gid_t gid, gid_t *groups
> > > ATTRIBUTE_UNUSED,
> > > }
> > >
> > > # if HAVE_SETGROUPS
> > > +VIR_DEBUG("setgroups %d max %d", ngroups, NGROUPS_MAX);
> > > if (gid != (gid_t)-1 && setgroups(ngroups, groups) < 0) {
> > > virReportSystemError(errno, "%s",
> > > _("cannot set supplemental groups"));
> > >
> > >
> >
> > Yes, there's an overflow:
> > 2019-08-21 18:25:37.943+: 123145413914624: debug : virSetUIDGID:1046 :
> > setgroups 23 max 16
> >
> > Related samba ticket (it also has references to the python and dovecot
> > issues):
> > https://bugzilla.samba.org/show_bug.cgi?id=8773
>
> The quick hack is to simply truncate groups to NGROUPS_MAX.
>
> Reading that bug, the proper fix looks quite alot more complex
Just to confirm that the root cause of the issue was the above block,
I bypassed the HAVE_SETGROUPS section completely on Darwin, and things
work as expected.
Where would be an appropriate place to track this bug?
Marcus.
--
Marcus Furlong
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] tests: qemublock: Use bigger numbers as dummy capacity/physical
On Fri, Aug 30, 2019 at 05:30:35PM +0200, Christophe de Dinechin wrote:
Peter Krempa writes:
Actually test that the full range is available.
[...]
return -1;
diff --git a/tests/qemublocktestdata/imagecreate/luks-encopts.json
b/tests/qemublocktestdata/imagecreate/luks-encopts.json
index f065ad89a7..c5ca863f5b 100644
--- a/tests/qemublocktestdata/imagecreate/luks-encopts.json
+++ b/tests/qemublocktestdata/imagecreate/luks-encopts.json
@@ -2,7 +2,7 @@ protocol:
{
"driver": "file",
"filename": "/var/lib/libvirt/images/i.img",
- "size": 42
+ "size": 4294967296
}
Does any of these tests actually require that much space to run?
No, parsing a four hundred byte json file does not really take much
space.
Jano
If so, what is the impact on QE teams?
--
Cheers,
Christophe de Dinechin (IRC c3d)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Fwd: libvirtd failing on MacOS in setgroups
On Fri, Aug 30, 2019 at 11:42:50AM -0400, Marcus Furlong wrote:
> On Thu, 22 Aug 2019 at 05:34, Daniel P. Berrangé wrote:
> >
> > On Wed, Aug 21, 2019 at 09:34:05PM +0300, Roman Bolshakov wrote:
> > > On Wed, Aug 21, 2019 at 05:55:51PM +0100, Daniel P. Berrangé wrote:
> > > > On Wed, Aug 21, 2019 at 12:47:03PM -0400, Marcus Furlong wrote:
> > > > > On Wed, 21 Aug 2019 at 08:23, Daniel P. Berrangé
> > > > > wrote:
> > > > > >
> > > > > > On Tue, Aug 20, 2019 at 11:11:07AM -0400, Marcus Furlong wrote:
> > > > > > > Resend to libvir-list in case that is more appropriate:
> > > > > > >
> > > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > I get the following error when running libvirtd on MacOS as root:
> > > > > > >
> > > > > > > 2019-07-11 00:12:33.673+: 123145573953536: error :
> > > > > > > qemuProcessQMPLaunch:8501 : internal error: Failed to start QEMU
> > > > > > > binary /usr/local/bin/qemu-system-x86_64 for probing: libvirt:
> > > > > > > error
> > > > > > > : cannot set supplemental groups: Invalid argument
> > > > > >
> > > > > > Are you able to run 'strace' (or whatever MacOS eqiuv is) to see
> > > > > > the values passed to setgroups when it fails ?
> > > > >
> > > > > I ran `dtruss -f -l -s /usr/local/sbin/libvirtd` but the setgroups
> > > > > calls seem to be missing.
> > > > >
> > > > > Looking at other sources, it seems like some have special treatment of
> > > > > setgroups on MacOS, e.g. samba:
> > > > >
> > > > >
> > > > > https://github.com/samba-team/samba/blob/v4-11-stable/source3/smbd/sec_ctx.c#L261-#L305
> > > > >
> > > > > Perhaps this is needed for libvirt?
> > > >
> > > > The capping of ngroups to NGROUPS_MAX looks like a possibe reason.
> > > >
> > > > Adding this debug might show us if we're exceeding it:
> > > >
> > > > diff --git a/src/util/virutil.c b/src/util/virutil.c
> > > > index 89d2cf011f..effc02b898 100644
> > > > --- a/src/util/virutil.c
> > > > +++ b/src/util/virutil.c
> > > > @@ -1043,6 +1043,7 @@ virSetUIDGID(uid_t uid, gid_t gid, gid_t *groups
> > > > ATTRIBUTE_UNUSED,
> > > > }
> > > >
> > > > # if HAVE_SETGROUPS
> > > > +VIR_DEBUG("setgroups %d max %d", ngroups, NGROUPS_MAX);
> > > > if (gid != (gid_t)-1 && setgroups(ngroups, groups) < 0) {
> > > > virReportSystemError(errno, "%s",
> > > > _("cannot set supplemental groups"));
> > > >
> > > >
> > >
> > > Yes, there's an overflow:
> > > 2019-08-21 18:25:37.943+: 123145413914624: debug : virSetUIDGID:1046
> > > : setgroups 23 max 16
> > >
> > > Related samba ticket (it also has references to the python and dovecot
> > > issues):
> > > https://bugzilla.samba.org/show_bug.cgi?id=8773
> >
> > The quick hack is to simply truncate groups to NGROUPS_MAX.
> >
> > Reading that bug, the proper fix looks quite alot more complex
>
> Just to confirm that the root cause of the issue was the above block,
> I bypassed the HAVE_SETGROUPS section completely on Darwin, and things
> work as expected.
>
> Where would be an appropriate place to track this bug?
You can file a BZ here:
https://libvirt.org/bugs.html
I would note, however, that very few libvirt maintainers use or have
access to macOS. So if you're able to try creating a viable patch
yourself that would maximise likelihood of it getting fixed in a
reasonable amount of time.
I'll happily review & give feedback on the approach, but I'm not
able to actively test myself, besides a plain compile-check we
get via Travis CI.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 03/16] domain: add rendernode attribute on
Hi
On Fri, Aug 23, 2019 at 9:05 PM Cole Robinson wrote:
>
> On 8/23/19 12:21 PM, Cole Robinson wrote:
> > From: Marc-André Lureau
> >
> > vhost-user-gpu helper may accept --render-node option to specify on
> > which GPU should the renderning be done.
> >
>
> What does it do if the user doesn't pass one? Pick one itself, or just
> not use one somehow?
This is left unspecified. So it may probe one itself and use it, or
fallback to full-software.
>
> If it picks one, then we may need to treat this like we treat other
> rendernode instances and autoallocate one if the user doesn't specify,
> otherwise we won't be able to add the path to the cgroup for example, or
> selinux label it if necessary. I'm not sure if that actually applies in
> this case, but it's worth considering.
>
I think we may want to auto-allocate a drm node if the helper accepts
the argument, as it is almost always what you'd want. But then we lose
the ability to *not* specify it. I am fine with that.
> > (by comparison rendernode is the target/display rendering)
> >
> > Signed-off-by: Marc-André Lureau
> > Signed-off-by: Cole Robinson
>
> Also I see now that I accidentally signed off all these patches, that
> was not intentional. Please strip those from v3
>
> > ---
> > docs/formatdomain.html.in | 5 +
> > docs/schemas/domaincommon.rng | 5 +
> > src/conf/domain_conf.c| 18 +-
> > src/conf/domain_conf.h| 1 +
> > 4 files changed, 28 insertions(+), 1 deletion(-)
> >
> > diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> > index ec650fbe17..5a4807d937 100644
> > --- a/docs/formatdomain.html.in
> > +++ b/docs/formatdomain.html.in
> > @@ -7059,6 +7059,11 @@ qemu-kvm -net nic,model=? /dev/null
> > Enable 3D acceleration (for vbox driver
> > since 0.7.1, qemu driver
> > since 1.3.0)
> > +
> > +rendernode
> > +Absolute path to a host's DRI device to be used for
> > +rendering (for vhost-user driver only, > +class="since">since 5.5.0)
> >
> >
> >
> > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> > index bac566855d..6e91fe6cef 100644
> > --- a/docs/schemas/domaincommon.rng
> > +++ b/docs/schemas/domaincommon.rng
> > @@ -3644,6 +3644,11 @@
> >
> >
> >
> > +
> > +
> > +
> > +
> > +
> >
> >
> >
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index f51575d57d..2cc055491d 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -2832,6 +2832,8 @@ virDomainVideoDefClear(virDomainVideoDefPtr def)
> >
> > virDomainDeviceInfoClear(&def->info);
> >
> > +if (def->accel)
> > +VIR_FREE(def->accel->rendernode);
> > VIR_FREE(def->accel);
> > VIR_FREE(def->virtio);
> > VIR_FREE(def->driver);
> > @@ -15270,6 +15272,7 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
> > int val;
> > VIR_AUTOFREE(char *) accel2d = NULL;
> > VIR_AUTOFREE(char *) accel3d = NULL;
> > +VIR_AUTOFREE(char *) rendernode = NULL;
> >
> > cur = node->children;
> > while (cur != NULL) {
> > @@ -15278,12 +15281,13 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
> > virXMLNodeNameEqual(cur, "acceleration")) {
> > accel3d = virXMLPropString(cur, "accel3d");
> > accel2d = virXMLPropString(cur, "accel2d");
> > +rendernode = virXMLPropString(cur, "rendernode");
> > }
> > }
> > cur = cur->next;
> > }
> >
> > -if (!accel3d && !accel2d)
> > +if (!accel3d && !accel2d && !rendernode)
> > return NULL;
> >
> > if (VIR_ALLOC(def) < 0)
> > @@ -15307,6 +15311,9 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
> > def->accel2d = val;
> > }
> >
> > +if (VIR_STRDUP(def->rendernode, rendernode) < 0)
> > +goto cleanup;
> > +
>
> Huh, this function has no error reporting? A bogus accel2d/accel3d value
> will raise an error but it never triggers the error path in the calling
> function. Not your fault of course :)
>
> > cleanup:
> > return def;
> > }
> > @@ -15474,6 +15481,11 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr
> > xmlopt,
> > goto error;
> > }
> > }
> > +if (!def->vhostuser && def->accel && def->accel->rendernode) {
> > +virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > + _("unsupported rendernode accel attribute without
> > 'vhost-user'"));
> > +goto error;
> > +}
> >
>
> This function doesn't represent best practices, but this style of check
> should be moved to virDomainVideoDefValidate IMO
>
> Thanks,
> Cole
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir
[libvirt] [PATCH] qemu: Validate arg in qemuAgentErrorComandUnsupported()
Coverity noted that 'reply' can be NULL after calling
qemuAgentCommand(). Avoid dereferencing reply in
qemuAgentErrorComandUnsupported() in that case.
Signed-off-by: Jonathon Jongsma
---
src/qemu/qemu_agent.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index fddedf5cb6..34e1a85d64 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -1005,7 +1005,12 @@ static bool
qemuAgentErrorCommandUnsupported(virJSONValuePtr reply)
{
const char *klass;
-virJSONValuePtr error = virJSONValueObjectGet(reply, "error");
+virJSONValuePtr error;
+
+if (!reply)
+return false;
+
+error = virJSONValueObjectGet(reply, "error");
if (!error)
return false;
--
2.21.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Fwd: libvirtd failing on MacOS in setgroups
[adding gnulib]
On 8/21/19 1:34 PM, Roman Bolshakov wrote:
> I get the following error when running libvirtd on MacOS as root:
>
> 2019-07-11 00:12:33.673+: 123145573953536: error :
> qemuProcessQMPLaunch:8501 : internal error: Failed to start QEMU
> binary /usr/local/bin/qemu-system-x86_64 for probing: libvirt: error
> : cannot set supplemental groups: Invalid argument
>> +++ b/src/util/virutil.c
>> @@ -1043,6 +1043,7 @@ virSetUIDGID(uid_t uid, gid_t gid, gid_t *groups
>> ATTRIBUTE_UNUSED,
>> }
>>
>> # if HAVE_SETGROUPS
>> +VIR_DEBUG("setgroups %d max %d", ngroups, NGROUPS_MAX);
>> if (gid != (gid_t)-1 && setgroups(ngroups, groups) < 0) {
>> virReportSystemError(errno, "%s",
>> _("cannot set supplemental groups"));
>>
>>
>
> Yes, there's an overflow:
> 2019-08-21 18:25:37.943+: 123145413914624: debug : virSetUIDGID:1046 :
> setgroups 23 max 16
>
> Related samba ticket (it also has references to the python and dovecot
> issues):
> https://bugzilla.samba.org/show_bug.cgi?id=8773
I wonder if gnulib could provide a workaround setgroups() that overcomes
this issue (it's better to maintain such a patch there, where it
benefits multiple programs, rather than just in libvirt).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 00/19] Add vhost-user-gpu support
From: Marc-André Lureau Hi, This series adds support for running virtio GPUs in seperate processes, thanks to vhost-user backend. The QEMU support landed for 4.1. There are several benefits of running the GPU/virgl in an external process, since Mesa is rather heavy on the qemu main loop, and may block for a while, or crash. The external GPU process is started with one end of a socket pair, the other end is given to a QEMU chardev attached to a device. The external process is also added to the cgroup to limit resources usage. vhost-user is a generic mechanism that allows implementing virtio device dataplane handling in a separate userspace process. vhost-user-gpu here notably moves the virgl 3d handling out of the main qemu process. The external process will be /usr/libexec/vhost-user-gpu, which comes from qemu.git contrib/vhost-user-gpu code, first released in qemu-4.1. Part of this series deals with discovering the location on disk of the vhost-user-gpu binary, and what capabilities it provides. This uses a similar mechanism to firmware.json, described in qemu docs/interop/vhost-user.json https://github.com/qemu/qemu/blob/master/docs/interop/vhost-user.json qemu 4.1 ships a 50-qemu-gpu.json to match. I believe virtio-fs will use a similar mechanism when it lands in upstream qemu, as virtiofsd is a separate process that communicates with qemu over vhost-user. For a bit more background on vhost-user-gpu process handling and the json interop motivation, here's some of the qemu discussion: https://lists.nongnu.org/archive/html/qemu-devel/2018-08/msg02610.html https://lists.nongnu.org/archive/html/qemu-devel/2018-09/msg00807.html For this series, the XML to enable this is: rendernode is optional qemu_vhost_user.c handles vhost-user.json qemu_vhost_user_gpu.c handles the process management for vhost-user-gpu v3: - rename qemu_configs -> qemu_interop_config - replace with - move vhost_user_binary to virDomainVideoDriverDef - some VIR_AUTO usage improvements - introduce qemuDomainVideoPrivate to store vhost-user fd - improved selection of -device model - use DO_TEST_CAPS_LATEST - allocate a rendernode with virHostGetDRMRenderNode() - but no clear idea how to have a test - add a patch to ignore EPERM in virDirOpenIfExists() - better domain checks/validate - added Ján r-b - no s-o-b from Cole, per request and commits taken from his git branch - rebase, indentation/style fixes, update doc, version.. v2: - rebase to master - if test file build by dropping LDADDS usage - syntax-check issues: * use #pragma once * if () bracket issues * jump label indent issues * error message %s usage * size_t for loops Marc-André Lureau (19): qemu: generalize qemuFetchConfigs util: ignore EACCESS in virDirOpenIfExists conf: format/parse/rng/docs for video domain: add rendernode attribute on qemu-cgroup: allow accel rendernode access qemu: add vhost-user-gpu capabilities checks qemu: check that qemu is vhost-user-vga capable qemu: validate virtio-gpu with vhost-user qemu: restrict 'virgl=' option to non-vhostuser video type qemu: add vhost-user helpers qemu: add qemuSecurityStartVhostUserGPU helper conf: add privateData to virDomainVideoDef qemu: add qemuDomainVideoPrivate qemu: add vhost-user-gpu helper unit qemu: prepare domain for vhost-user GPU qemu: start/stop the vhost-user-gpu external device qemu: build vhost-user GPU devices tests: mock execv/execve tests: add vhost-user-gpu xml2argv tests docs/formatdomain.html.in | 18 +- docs/schemas/domaincommon.rng | 13 + src/conf/domain_conf.c| 87 +++- src/conf/domain_conf.h| 21 +- src/qemu/Makefile.inc.am | 6 + src/qemu/qemu_capabilities.c | 4 + src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_cgroup.c| 24 + src/qemu/qemu_command.c | 65 ++- src/qemu/qemu_domain.c| 50 ++- src/qemu/qemu_domain.h| 14 +- src/qemu/qemu_extdevice.c | 73 ++- src/qemu/qemu_extdevice.h | 5 + src/qemu/qemu_firmware.c | 144 +- src/qemu/qemu_interop_config.c| 183 src/qemu/qemu_interop_config.h| 25 ++ src/qemu/qemu_process.c | 61 ++- src/qemu/qemu_security.c | 40 ++ src/qemu/qemu_security.h | 6 + src/qemu/qemu_vhost_user.c| 422 ++ src/qemu/qemu_vhost_user.h| 48 ++ src/qemu/qemu_vhost_user_gpu.c| 285 src/qemu/qemu_vhost_user_gpu.h| 50 +++ src/util/virfile.c| 13 +- tests/Makefile.am | 9 + .../caps_4.1.
[libvirt] [PATCH v3 01/19] qemu: generalize qemuFetchConfigs
From: Marc-André Lureau
The same config files disovery & priority rules are used for
vhost-user backends.
No functional change, the only difference is that
qemuInteropFetchConfigs() takes a "name" argument and construct paths
with it (ex: "firmware").
Signed-off-by: Marc-André Lureau
---
src/qemu/Makefile.inc.am | 2 +
src/qemu/qemu_firmware.c | 144 +-
src/qemu/qemu_interop_config.c | 183 +
src/qemu/qemu_interop_config.h | 25 +
4 files changed, 212 insertions(+), 142 deletions(-)
create mode 100644 src/qemu/qemu_interop_config.c
create mode 100644 src/qemu/qemu_interop_config.h
diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
index d16b315ebc..a79e957f8d 100644
--- a/src/qemu/Makefile.inc.am
+++ b/src/qemu/Makefile.inc.am
@@ -30,6 +30,8 @@ QEMU_DRIVER_SOURCES = \
qemu/qemu_hotplugpriv.h \
qemu/qemu_conf.c \
qemu/qemu_conf.h \
+ qemu/qemu_interop_config.c \
+ qemu/qemu_interop_config.h \
qemu/qemu_process.c \
qemu/qemu_process.h \
qemu/qemu_processpriv.h \
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 983a7c83b2..080ac10417 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -23,13 +23,12 @@
#include
#include "qemu_firmware.h"
+#include "qemu_interop_config.h"
#include "configmake.h"
#include "qemu_capabilities.h"
#include "qemu_domain.h"
#include "qemu_process.h"
#include "virarch.h"
-#include "virfile.h"
-#include "virhash.h"
#include "virjson.h"
#include "virlog.h"
#include "virstring.h"
@@ -907,150 +906,11 @@ qemuFirmwareFormat(qemuFirmwarePtr fw)
}
-static int
-qemuFirmwareBuildFileList(virHashTablePtr files, const char *dir)
-{
-DIR *dirp;
-struct dirent *ent = NULL;
-int rc;
-int ret = -1;
-
-if ((rc = virDirOpenIfExists(&dirp, dir)) < 0)
-return -1;
-
-if (rc == 0)
-return 0;
-
-while ((rc = virDirRead(dirp, &ent, dir)) > 0) {
-VIR_AUTOFREE(char *) filename = NULL;
-VIR_AUTOFREE(char *) path = NULL;
-struct stat sb;
-
-if (STRPREFIX(ent->d_name, "."))
-continue;
-
-if (VIR_STRDUP(filename, ent->d_name) < 0)
-goto cleanup;
-
-if (virAsprintf(&path, "%s/%s", dir, filename) < 0)
-goto cleanup;
-
-if (stat(path, &sb) < 0) {
-virReportSystemError(errno, _("Unable to access %s"), path);
-goto cleanup;
-}
-
-if (!S_ISREG(sb.st_mode) && !S_ISLNK(sb.st_mode))
-continue;
-
-if (virHashUpdateEntry(files, filename, path) < 0)
-goto cleanup;
-
-path = NULL;
-}
-
-if (rc < 0)
-goto cleanup;
-
-ret = 0;
- cleanup:
-virDirClose(&dirp);
-return ret;
-}
-
-static int
-qemuFirmwareFilesSorter(const virHashKeyValuePair *a,
-const virHashKeyValuePair *b)
-{
-return strcmp(a->key, b->key);
-}
-
-#define QEMU_FIRMWARE_SYSTEM_LOCATION PREFIX "/share/qemu/firmware"
-#define QEMU_FIRMWARE_ETC_LOCATION SYSCONFDIR "/qemu/firmware"
-
int
qemuFirmwareFetchConfigs(char ***firmwares,
bool privileged)
{
-VIR_AUTOPTR(virHashTable) files = NULL;
-VIR_AUTOFREE(char *) homeConfig = NULL;
-VIR_AUTOFREE(char *) xdgConfig = NULL;
-VIR_AUTOFREE(virHashKeyValuePairPtr) pairs = NULL;
-virHashKeyValuePairPtr tmp = NULL;
-
-*firmwares = NULL;
-
-if (!privileged) {
-/* This is a slight divergence from the specification.
- * Since the system daemon runs as root, it doesn't make
- * much sense to parse files in root's home directory. It
- * makes sense only for session daemon which runs under
- * regular user. */
-if (VIR_STRDUP(xdgConfig, getenv("XDG_CONFIG_HOME")) < 0)
-return -1;
-
-if (!xdgConfig) {
-VIR_AUTOFREE(char *) home = virGetUserDirectory();
-
-if (!home)
-return -1;
-
-if (virAsprintf(&xdgConfig, "%s/.config", home) < 0)
-return -1;
-}
-
-if (virAsprintf(&homeConfig, "%s/qemu/firmware", xdgConfig) < 0)
-return -1;
-}
-
-if (!(files = virHashCreate(10, virHashValueFree)))
-return -1;
-
-if (qemuFirmwareBuildFileList(files, QEMU_FIRMWARE_SYSTEM_LOCATION) < 0)
-return -1;
-
-if (qemuFirmwareBuildFileList(files, QEMU_FIRMWARE_ETC_LOCATION) < 0)
-return -1;
-
-if (homeConfig &&
-qemuFirmwareBuildFileList(files, homeConfig) < 0)
-return -1;
-
-/* At this point, the @files hash table contains unique set of filenames
- * where each filename (as key) has the highest priority full pathname
- * associated with it. */
-
-if (virHashSize(files) == 0)
-return 0;
-
-if (!(pairs = virHashGetItems(files, qemuFirmwareFilesSorter)))
-return -1;
-
-
[libvirt] [PATCH v3 06/19] qemu: add vhost-user-gpu capabilities checks
From: Marc-André Lureau
Those new devices are available since QEMU 4.1.
Signed-off-by: Marc-André Lureau
Reviewed-by: Ján Tomko
---
src/qemu/qemu_capabilities.c | 4
src/qemu/qemu_capabilities.h | 2 ++
tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml | 2 ++
3 files changed, 8 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 73300128ea..d27f54d16e 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -537,6 +537,8 @@ VIR_ENUM_IMPL(virQEMUCaps,
/* 335 */
"bochs-display",
"migration-file-drop-cache",
+ "vhost-user-gpu",
+ "vhost-user-vga",
);
@@ -1127,6 +1129,8 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ "virtio-serial-pci-non-transitional", QEMU_CAPS_VIRTIO_PCI_TRANSITIONAL
},
{ "max-x86_64-cpu", QEMU_CAPS_X86_MAX_CPU },
{ "bochs-display", QEMU_CAPS_DEVICE_BOCHS_DISPLAY },
+{ "vhost-user-gpu", QEMU_CAPS_DEVICE_VHOST_USER_GPU },
+{ "vhost-user-vga", QEMU_CAPS_DEVICE_VHOST_USER_VGA },
};
static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsVirtioBalloon[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 68ef6c49b4..6f1e1dd2f3 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -518,6 +518,8 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
syntax-check */
/* 335 */
QEMU_CAPS_DEVICE_BOCHS_DISPLAY, /* -device bochs-display */
QEMU_CAPS_MIGRATION_FILE_DROP_CACHE, /* migration with disk cache on is
safe for type='file' disks */
+QEMU_CAPS_DEVICE_VHOST_USER_GPU, /* -device vhost-user-gpu */
+QEMU_CAPS_DEVICE_VHOST_USER_VGA, /* -device vhost-user-vga */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
index f4583d7fe7..5d4540b3f7 100644
--- a/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml
@@ -209,6 +209,8 @@
+
+
450
0
43100759
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 03/19] conf: format/parse/rng/docs for video
From: Marc-André Lureau Accept a new driver name attribute to specify usage of helper process, ex: Signed-off-by: Marc-André Lureau --- docs/formatdomain.html.in | 12 ++- docs/schemas/domaincommon.rng | 8 + src/conf/domain_conf.c| 42 ++- src/conf/domain_conf.h| 12 +++ tests/qemuxml2argvdata/virtio-options.xml | 2 +- 5 files changed, 73 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index fcb7c59c00..f56d04eac3 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -6969,6 +6969,7 @@ qemu-kvm -net nic,model=? /dev/null+ ... @@ -7069,7 +7070,16 @@ qemu-kvm -net nic,model=? /dev/null The subelement driver can be used to tune the device: - virtio options + name + +Specify the backend driver to use, either "qemu" or +"vhostuser" depending on the hypervisor features available +(since 5.8.0). "qemu" is the +default QEMU backend. "vhostuser" will use a separate +vhost-user process backend (for virtio +device). + +virtio options Virtio-specific options can also be set (Since 3.5.0) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index c48f8c4f56..7e0b63f0a6 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3560,6 +3560,14 @@ + + + +qemu +vhostuser + + + diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b7a342bb91..dc5b7eac5c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -728,6 +728,13 @@ VIR_ENUM_IMPL(virDomainPanicModel, "s390", ); +VIR_ENUM_IMPL(virDomainVideoBackend, + VIR_DOMAIN_VIDEO_BACKEND_TYPE_LAST, + "default", + "qemu", + "vhostuser", +); + VIR_ENUM_IMPL(virDomainVideo, VIR_DOMAIN_VIDEO_TYPE_LAST, "default", @@ -6249,6 +6256,23 @@ virDomainVideoDefValidate(const virDomainVideoDef *video, } } +switch (video->backend) { +case VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER: +if (video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'vhostuser' driver is only supported with 'virtio' device")); +return -1; +} +break; +case VIR_DOMAIN_VIDEO_BACKEND_TYPE_DEFAULT: +case VIR_DOMAIN_VIDEO_BACKEND_TYPE_QEMU: +break; +case VIR_DOMAIN_VIDEO_BACKEND_TYPE_LAST: +default: +virReportEnumRangeError(virDomainInputType, video->backend); +return -1; +} + return 0; } @@ -15358,6 +15382,7 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt, xmlNodePtr cur; VIR_XPATH_NODE_AUTORESTORE(ctxt); VIR_AUTOFREE(char *) type = NULL; +VIR_AUTOFREE(char *) driver_name = NULL; VIR_AUTOFREE(char *) heads = NULL; VIR_AUTOFREE(char *) vram = NULL; VIR_AUTOFREE(char *) vram64 = NULL; @@ -15393,6 +15418,7 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt, if (virXMLNodeNameEqual(cur, "driver")) { if (virDomainVirtioOptionsParseXML(cur, &def->virtio) < 0) goto error; +driver_name = virXMLPropString(cur, "name"); } } cur = cur->next; @@ -15408,6 +15434,16 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt, def->type = virDomainVideoDefaultType(dom); } +if (driver_name) { +if ((def->backend = virDomainVideoBackendTypeFromString(driver_name)) < 0) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown video driver '%s'"), driver_name); +goto error; +} +} else { +def->backend = VIR_DOMAIN_VIDEO_BACKEND_TYPE_DEFAULT; +} + if (ram) { if (def->type != VIR_DOMAIN_VIDEO_TYPE_QXL) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -26463,13 +26499,17 @@ virDomainVideoDefFormat(virBufferPtr buf, virDomainVirtioOptionsFormat(&driverBuf, def->virtio); if (virBufferCheckError(&driverBuf) < 0) goto cleanup; -if (virBufferUse(&driverBuf) || (def->driver && def->driver->vgaconf)) { +if (virBufferUse(&driverBuf) || (def->driver && def->driver->vgaconf) || +def->backend != VIR_DOMAIN_VIDEO_BACKEND_TYPE_DEFAULT) { virBufferAddLit(buf, "driver && def->
[libvirt] [PATCH v3 04/19] domain: add rendernode attribute on
From: Marc-André Lureau
vhost-user-gpu helper takes --render-node option to specify on which
GPU should the renderning be done.
Signed-off-by: Marc-André Lureau
---
docs/formatdomain.html.in | 6 ++
docs/schemas/domaincommon.rng | 5 +
src/conf/domain_conf.c| 17 -
src/conf/domain_conf.h| 1 +
tests/qemuxml2argvdata/virtio-options.xml | 2 +-
5 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index f56d04eac3..5b2b1a71b1 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -7054,6 +7054,12 @@ qemu-kvm -net nic,model=? /dev/null
Enable 3D acceleration (for vbox driver
since 0.7.1, qemu driver
since 1.3.0)
+
+rendernode
+Absolute path to a host's DRI device to be used for
+rendering (for 'vhostuser' driver only, since 5.8.0). If none is specified,
+libvirt will pick one available.
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 7e0b63f0a6..f4a1074080 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -3643,6 +3643,11 @@
+
+
+
+
+
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index dc5b7eac5c..129651085f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2839,6 +2839,8 @@ virDomainVideoDefClear(virDomainVideoDefPtr def)
virDomainDeviceInfoClear(&def->info);
+if (def->accel)
+VIR_FREE(def->accel->rendernode);
VIR_FREE(def->accel);
VIR_FREE(def->virtio);
VIR_FREE(def->driver);
@@ -6266,6 +6268,11 @@ virDomainVideoDefValidate(const virDomainVideoDef *video,
break;
case VIR_DOMAIN_VIDEO_BACKEND_TYPE_DEFAULT:
case VIR_DOMAIN_VIDEO_BACKEND_TYPE_QEMU:
+if (video->accel && video->accel->rendernode) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("unsupported rendernode accel attribute without
'vhostuser'"));
+return -1;
+}
break;
case VIR_DOMAIN_VIDEO_BACKEND_TYPE_LAST:
default:
@@ -15294,6 +15301,7 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
int val;
VIR_AUTOFREE(char *) accel2d = NULL;
VIR_AUTOFREE(char *) accel3d = NULL;
+VIR_AUTOFREE(char *) rendernode = NULL;
cur = node->children;
while (cur != NULL) {
@@ -15302,12 +15310,13 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
virXMLNodeNameEqual(cur, "acceleration")) {
accel3d = virXMLPropString(cur, "accel3d");
accel2d = virXMLPropString(cur, "accel2d");
+rendernode = virXMLPropString(cur, "rendernode");
}
}
cur = cur->next;
}
-if (!accel3d && !accel2d)
+if (!accel3d && !accel2d && !rendernode)
return NULL;
if (VIR_ALLOC(def) < 0)
@@ -15331,6 +15340,8 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
def->accel2d = val;
}
+VIR_STEAL_PTR(def->rendernode, rendernode);
+
cleanup:
return def;
}
@@ -26476,6 +26487,10 @@ virDomainVideoAccelDefFormat(virBufferPtr buf,
virBufferAsprintf(buf, " accel2d='%s'",
virTristateBoolTypeToString(def->accel2d));
}
+if (def->rendernode) {
+virBufferAsprintf(buf, " rendernode='%s'",
+ def->rendernode);
+}
virBufferAddLit(buf, "/>\n");
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index af7fec6e22..0d7cb0b6fa 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1415,6 +1415,7 @@ VIR_ENUM_DECL(virDomainVideoVGAConf);
struct _virDomainVideoAccelDef {
int accel2d; /* enum virTristateBool */
int accel3d; /* enum virTristateBool */
+char *rendernode;
};
diff --git a/tests/qemuxml2argvdata/virtio-options.xml
b/tests/qemuxml2argvdata/virtio-options.xml
index bdfadca22d..dd9a4f4a01 100644
--- a/tests/qemuxml2argvdata/virtio-options.xml
+++ b/tests/qemuxml2argvdata/virtio-options.xml
@@ -75,7 +75,7 @@
-
+
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 02/19] util: ignore EACCESS in virDirOpenIfExists
From: Marc-André Lureau
Whether a directory exists or is not readable shouldn't make a big
diffence.
This removes errors when firmare or vhost-user config is looked up
from a user session libvirt if /etc/libvirt is not readable.
Signed-off-by: Marc-André Lureau
---
src/util/virfile.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 81a3c096eb..e830a9ca10 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2833,7 +2833,8 @@ virFileRemove(const char *path,
#endif /* WIN32 */
static int
-virDirOpenInternal(DIR **dirp, const char *name, bool ignoreENOENT, bool quiet)
+virDirOpenInternal(DIR **dirp, const char *name,
+ bool ignoreENOENT, bool ignoreEACCESS, bool quiet)
{
*dirp = opendir(name); /* exempt from syntax-check */
if (!*dirp) {
@@ -2842,6 +2843,8 @@ virDirOpenInternal(DIR **dirp, const char *name, bool
ignoreENOENT, bool quiet)
if (ignoreENOENT && errno == ENOENT)
return 0;
+if (ignoreEACCESS && errno == EACCES)
+return 0;
virReportSystemError(errno, _("cannot open directory '%s'"), name);
return -1;
}
@@ -2859,7 +2862,7 @@ virDirOpenInternal(DIR **dirp, const char *name, bool
ignoreENOENT, bool quiet)
int
virDirOpen(DIR **dirp, const char *name)
{
-return virDirOpenInternal(dirp, name, false, false);
+return virDirOpenInternal(dirp, name, false, false, false);
}
/**
@@ -2868,13 +2871,13 @@ virDirOpen(DIR **dirp, const char *name)
* @name: path of the directory
*
* Returns 1 on success.
- * If opendir returns ENOENT, 0 is returned without reporting an error.
+ * If opendir returns ENOENT or EACCES, 0 is returned without reporting an
error.
* On other errors, -1 is returned and an error is reported.
*/
int
virDirOpenIfExists(DIR **dirp, const char *name)
{
-return virDirOpenInternal(dirp, name, true, false);
+return virDirOpenInternal(dirp, name, true, true, false);
}
/**
@@ -2890,7 +2893,7 @@ virDirOpenIfExists(DIR **dirp, const char *name)
int
virDirOpenQuiet(DIR **dirp, const char *name)
{
-return virDirOpenInternal(dirp, name, false, true);
+return virDirOpenInternal(dirp, name, false, false, true);
}
/**
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 05/19] qemu-cgroup: allow accel rendernode access
From: Marc-André Lureau
Signed-off-by: Marc-André Lureau
Reviewed-by: Ján Tomko
---
src/qemu/qemu_cgroup.c | 24
1 file changed, 24 insertions(+)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index ecd96efb0a..eb6f993d8e 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -503,6 +503,25 @@ qemuSetupGraphicsCgroup(virDomainObjPtr vm,
}
+static int
+qemuSetupVideoAccelCgroup(virDomainObjPtr vm,
+ virDomainVideoAccelDefPtr def)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+int ret;
+
+if (!def->rendernode ||
+!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_DEVICES))
+return 0;
+
+ret = virCgroupAllowDevicePath(priv->cgroup, def->rendernode,
+ VIR_CGROUP_DEVICE_RW, false);
+virDomainAuditCgroupPath(vm, priv->cgroup, "allow", def->rendernode,
+ "rw", ret);
+return ret;
+}
+
+
static int
qemuSetupBlkioCgroup(virDomainObjPtr vm)
{
@@ -803,6 +822,11 @@ qemuSetupDevicesCgroup(virDomainObjPtr vm)
goto cleanup;
}
+for (i = 0; i < vm->def->nvideos; i++) {
+if (qemuSetupVideoAccelCgroup(vm, vm->def->videos[i]->accel) < 0)
+goto cleanup;
+}
+
for (i = 0; i < vm->def->ninputs; i++) {
if (qemuSetupInputCgroup(vm, vm->def->inputs[i]) < 0)
goto cleanup;
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 08/19] qemu: validate virtio-gpu with vhost-user
From: Marc-André Lureau
Check qemu capability, and accept 3d acceleration. 3d acceleration
support is checked when looking for a suitable vhost-user helper.
Signed-off-by: Marc-André Lureau
---
src/qemu/qemu_process.c | 57 +++-
tests/qemuxml2argvtest.c | 3 +--
2 files changed, 34 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index c9921646e9..3fe46a7c7b 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -5266,34 +5266,43 @@ qemuProcessStartValidateVideo(virDomainObjPtr vm,
for (i = 0; i < vm->def->nvideos; i++) {
video = vm->def->videos[i];
-if ((video->type == VIR_DOMAIN_VIDEO_TYPE_VGA &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA)) ||
-(video->type == VIR_DOMAIN_VIDEO_TYPE_CIRRUS &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA)) ||
-(video->type == VIR_DOMAIN_VIDEO_TYPE_VMVGA &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMWARE_SVGA)) ||
-(video->type == VIR_DOMAIN_VIDEO_TYPE_QXL &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QXL)) ||
-(video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU)) ||
-(video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO &&
- video->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW &&
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW))) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("this QEMU does not support '%s' video device"),
- virDomainVideoTypeToString(video->type));
-return -1;
-}
-
-if (video->accel) {
-if (video->accel->accel3d == VIR_TRISTATE_SWITCH_ON &&
-(video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO ||
- !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_GPU_VIRGL))) {
+if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) {
+if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO &&
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VHOST_USER_GPU)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("this QEMU does not support 'vhost-user'
video device"));
+return -1;
+}
+} else {
+if ((video->type == VIR_DOMAIN_VIDEO_TYPE_VGA &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VGA)) ||
+(video->type == VIR_DOMAIN_VIDEO_TYPE_CIRRUS &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_CIRRUS_VGA)) ||
+(video->type == VIR_DOMAIN_VIDEO_TYPE_VMVGA &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VMWARE_SVGA)) ||
+(video->type == VIR_DOMAIN_VIDEO_TYPE_QXL &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QXL)) ||
+(video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU)) ||
+(video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO &&
+ video->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW))) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("%s 3d acceleration is not supported"),
+ _("this QEMU does not support '%s' video
device"),
virDomainVideoTypeToString(video->type));
return -1;
}
+
+if (video->accel) {
+if (video->accel->accel3d == VIR_TRISTATE_SWITCH_ON &&
+(video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO ||
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_GPU_VIRGL))) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("%s 3d acceleration is not supported"),
+ virDomainVideoTypeToString(video->type));
+return -1;
+}
+}
}
}
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 9395cc19a2..c6b10a08fb 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2874,8 +2874,7 @@ mymain(void)
DO_TEST("virtio-options", QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_VIRTIO_KEYBOARD,
QEMU_CAPS_VIRTIO_MOUSE, QEMU_CAPS_VIRTIO_TABLET,
QEMU_CAPS_VIRTIO_INPUT_HOST,
-QEMU_CAPS_DEVICE_VIRTIO_GPU,
-QEMU_CAPS_VIRTIO_GPU_VIRGL,
+QEMU_CAPS_DEVICE_VHOST_USER_GPU,
QEMU_CAPS_DEVICE_VIRTIO_RNG,
QEMU_CAPS_OBJECT_RNG_RANDOM,
QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo
[libvirt] [PATCH v3 07/19] qemu: check that qemu is vhost-user-vga capable
From: Marc-André Lureau
To support virtio VGA with vhost-user, vhost-user-vga device is necessary.
Signed-off-by: Marc-André Lureau
Reviewed-by: Ján Tomko
---
src/qemu/qemu_domain.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 657f3ecfe4..f694cf8bbc 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -12418,9 +12418,14 @@ bool
qemuDomainSupportsVideoVga(virDomainVideoDefPtr video,
virQEMUCapsPtr qemuCaps)
{
-if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO &&
-!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_VGA))
-return false;
+if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO) {
+if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VHOST_USER_VGA))
+return false;
+} else if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIRTIO_VGA)) {
+return false;
+}
+}
return true;
}
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 09/19] qemu: restrict 'virgl=' option to non-vhostuser video type
From: Marc-André Lureau
vhost-user device doesn't have a virgl option, it is passed to the
vhost-user-gpu helper process instead.
Signed-off-by: Marc-André Lureau
---
src/qemu/qemu_command.c| 9 ++---
tests/qemuxml2argvdata/virtio-options.args | 2 +-
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1ca1ecd2f0..e232406ff9 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4548,9 +4548,12 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
virBufferAsprintf(&buf, ",id=%s", video->info.alias);
-if (video->accel && video->accel->accel3d == VIR_TRISTATE_SWITCH_ON) {
-virBufferAsprintf(&buf, ",virgl=%s",
-
virTristateSwitchTypeToString(video->accel->accel3d));
+if (video->backend != VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER &&
+video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO) {
+if (video->accel && video->accel->accel3d == VIR_TRISTATE_SWITCH_ON) {
+virBufferAsprintf(&buf, ",virgl=%s",
+
virTristateSwitchTypeToString(video->accel->accel3d));
+}
}
if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL) {
diff --git a/tests/qemuxml2argvdata/virtio-options.args
b/tests/qemuxml2argvdata/virtio-options.args
index 40f85fdaa4..69dc1c73e5 100644
--- a/tests/qemuxml2argvdata/virtio-options.args
+++ b/tests/qemuxml2argvdata/virtio-options.args
@@ -49,7 +49,7 @@ ats=on \
ats=on \
-device virtio-input-host-pci,id=input3,evdev=/dev/input/event1234,bus=pci.0,\
addr=0x12,iommu_platform=on,ats=on \
--device
virtio-gpu-pci,id=video0,virgl=on,bus=pci.0,addr=0x2,iommu_platform=on,\
+-device virtio-gpu-pci,id=video0,bus=pci.0,addr=0x2,iommu_platform=on,\
ats=on \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0xc,iommu_platform=on,\
ats=on \
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 10/19] qemu: add vhost-user helpers
From: Marc-André Lureau
Add qemuVhostUserFetchConfigs() to discover vhost-user helpers.
qemuVhostUserFillDomainGPU() will find the first matching GPU helper
with the required capabilities and set the associated
vhost_user_binary.
Signed-off-by: Marc-André Lureau
---
src/conf/domain_conf.c| 2 +
src/conf/domain_conf.h| 1 +
src/qemu/Makefile.inc.am | 2 +
src/qemu/qemu_vhost_user.c| 422 ++
src/qemu/qemu_vhost_user.h| 48 ++
tests/Makefile.am | 9 +
.../etc/qemu/vhost-user/40-gpu.json | 1 +
.../etc/qemu/vhost-user/50-gpu.json | 0
.../qemu/vhost-user/test-vhost-user-gpu | 11 +
.../usr/share/qemu/vhost-user/30-gpu.json | 1 +
.../usr/share/qemu/vhost-user/50-gpu.json | 8 +
.../usr/share/qemu/vhost-user/60-gpu.json | 1 +
tests/qemuvhostusertest.c | 132 ++
13 files changed, 638 insertions(+)
create mode 100644 src/qemu/qemu_vhost_user.c
create mode 100644 src/qemu/qemu_vhost_user.h
create mode 12 tests/qemuvhostuserdata/etc/qemu/vhost-user/40-gpu.json
create mode 100644 tests/qemuvhostuserdata/etc/qemu/vhost-user/50-gpu.json
create mode 100755
tests/qemuvhostuserdata/usr/libexec/qemu/vhost-user/test-vhost-user-gpu
create mode 12
tests/qemuvhostuserdata/usr/share/qemu/vhost-user/30-gpu.json
create mode 100644
tests/qemuvhostuserdata/usr/share/qemu/vhost-user/50-gpu.json
create mode 12
tests/qemuvhostuserdata/usr/share/qemu/vhost-user/60-gpu.json
create mode 100644 tests/qemuvhostusertest.c
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 129651085f..270f597e3b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2843,6 +2843,8 @@ virDomainVideoDefClear(virDomainVideoDefPtr def)
VIR_FREE(def->accel->rendernode);
VIR_FREE(def->accel);
VIR_FREE(def->virtio);
+if (def->driver)
+VIR_FREE(def->driver->vhost_user_binary);
VIR_FREE(def->driver);
memset(def, 0, sizeof(*def));
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0d7cb0b6fa..faeb060c04 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1421,6 +1421,7 @@ struct _virDomainVideoAccelDef {
struct _virDomainVideoDriverDef {
virDomainVideoVGAConf vgaconf;
+char *vhost_user_binary;
};
struct _virDomainVideoDef {
diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
index a79e957f8d..aa22c1d2a7 100644
--- a/src/qemu/Makefile.inc.am
+++ b/src/qemu/Makefile.inc.am
@@ -60,6 +60,8 @@ QEMU_DRIVER_SOURCES = \
qemu/qemu_qapi.h \
qemu/qemu_tpm.c \
qemu/qemu_tpm.h \
+ qemu/qemu_vhost_user.c \
+ qemu/qemu_vhost_user.h \
$(NULL)
diff --git a/src/qemu/qemu_vhost_user.c b/src/qemu/qemu_vhost_user.c
new file mode 100644
index 00..bdc6cfb104
--- /dev/null
+++ b/src/qemu/qemu_vhost_user.c
@@ -0,0 +1,422 @@
+/*
+ * qemu_vhost_user.c: QEMU vhost-user
+ *
+ * Copyright (C) 2019 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ *
[libvirt] [PATCH v3 11/19] qemu: add qemuSecurityStartVhostUserGPU helper
From: Marc-André Lureau
See function documentation. Used in a following patch.
Signed-off-by: Marc-André Lureau
---
src/qemu/qemu_security.c | 40
src/qemu/qemu_security.h | 6 ++
2 files changed, 46 insertions(+)
diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c
index 3cd6d9bd3d..88eb2fb3a5 100644
--- a/src/qemu/qemu_security.c
+++ b/src/qemu/qemu_security.c
@@ -433,6 +433,46 @@ qemuSecurityRestoreChardevLabel(virQEMUDriverPtr driver,
}
+/*
+ * qemuSecurityStartVhostUserGPU:
+ *
+ * @driver: the QEMU driver
+ * @vm: the domain object
+ * @cmd: the command to run
+ * @existstatus: pointer to int returning exit status of process
+ * @cmdret: pointer to int returning result of virCommandRun
+ *
+ * Start the vhost-user-gpu process with approriate labels.
+ * This function returns -1 on security setup error, 0 if all the
+ * setup was done properly. In case the virCommand failed to run
+ * 0 is returned but cmdret is set appropriately with the process
+ * exitstatus also set.
+ */
+int
+qemuSecurityStartVhostUserGPU(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virCommandPtr cmd,
+ int *exitstatus,
+ int *cmdret)
+{
+if (virSecurityManagerSetChildProcessLabel(driver->securityManager,
+ vm->def, cmd) < 0)
+return -1;
+
+if (virSecurityManagerPreFork(driver->securityManager) < 0)
+return -1;
+
+*cmdret = virCommandRun(cmd, exitstatus);
+
+virSecurityManagerPostFork(driver->securityManager);
+
+if (*cmdret < 0)
+return -1;
+
+return 0;
+}
+
+
/*
* qemuSecurityStartTPMEmulator:
*
diff --git a/src/qemu/qemu_security.h b/src/qemu/qemu_security.h
index 68e377f418..a48ed8ec78 100644
--- a/src/qemu/qemu_security.h
+++ b/src/qemu/qemu_security.h
@@ -77,6 +77,12 @@ int qemuSecurityRestoreChardevLabel(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainChrDefPtr chr);
+int qemuSecurityStartVhostUserGPU(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virCommandPtr cmd,
+ int *exitstatus,
+ int *cmdret);
+
int qemuSecurityStartTPMEmulator(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virCommandPtr cmd,
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 12/19] conf: add privateData to virDomainVideoDef
From: Marc-André Lureau
Signed-off-by: Marc-André Lureau
---
src/conf/domain_conf.c | 26 +-
src/conf/domain_conf.h | 7 +--
2 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 270f597e3b..a482e2f88f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2819,13 +2819,19 @@ void virDomainShmemDefFree(virDomainShmemDefPtr def)
virDomainVideoDefPtr
-virDomainVideoDefNew(void)
+virDomainVideoDefNew(virDomainXMLOptionPtr xmlopt)
{
virDomainVideoDefPtr def;
if (VIR_ALLOC(def) < 0)
return NULL;
+if (xmlopt && xmlopt->privateData.videoNew &&
+!(def->privateData = xmlopt->privateData.videoNew())) {
+VIR_FREE(def);
+return NULL;
+}
+
def->heads = 1;
return def;
}
@@ -2857,6 +2863,7 @@ void virDomainVideoDefFree(virDomainVideoDefPtr def)
return;
virDomainVideoDefClear(def);
+virObjectUnref(def->privateData);
VIR_FREE(def);
}
@@ -5676,7 +5683,8 @@ virDomainDefPostParseVideo(virDomainDefPtr def,
static int
virDomainDefPostParseCommon(virDomainDefPtr def,
-struct virDomainDefPostParseDeviceIteratorData
*data)
+struct virDomainDefPostParseDeviceIteratorData
*data,
+virDomainXMLOptionPtr xmlopt)
{
size_t i;
@@ -5711,7 +5719,7 @@ virDomainDefPostParseCommon(virDomainDefPtr def,
if (virDomainDefPostParseTimer(def) < 0)
return -1;
-if (virDomainDefAddImplicitDevices(def) < 0)
+if (virDomainDefAddImplicitDevices(def, xmlopt) < 0)
return -1;
if (virDomainDefPostParseVideo(def, data) < 0)
@@ -5837,7 +5845,7 @@ virDomainDefPostParse(virDomainDefPtr def,
if (virDomainDefPostParseCheckFailure(def, parseFlags, ret) < 0)
goto cleanup;
-if ((ret = virDomainDefPostParseCommon(def, &data)) < 0)
+if ((ret = virDomainDefPostParseCommon(def, &data, xmlopt)) < 0)
goto cleanup;
if (xmlopt->config.assignAddressesCallback) {
@@ -15403,7 +15411,7 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt,
VIR_AUTOFREE(char *) vgamem = NULL;
VIR_AUTOFREE(char *) primary = NULL;
-if (!(def = virDomainVideoDefNew()))
+if (!(def = virDomainVideoDefNew(xmlopt)))
return NULL;
ctxt->node = node;
@@ -23721,7 +23729,7 @@ virDomainDefAddImplicitControllers(virDomainDefPtr def)
}
static int
-virDomainDefAddImplicitVideo(virDomainDefPtr def)
+virDomainDefAddImplicitVideo(virDomainDefPtr def, virDomainXMLOptionPtr xmlopt)
{
int ret = -1;
virDomainVideoDefPtr video = NULL;
@@ -23731,7 +23739,7 @@ virDomainDefAddImplicitVideo(virDomainDefPtr def)
if (def->ngraphics == 0 || def->nvideos > 0)
return 0;
-if (!(video = virDomainVideoDefNew()))
+if (!(video = virDomainVideoDefNew(xmlopt)))
goto cleanup;
video->type = virDomainVideoDefaultType(def);
if (VIR_APPEND_ELEMENT(def->videos, def->nvideos, video) < 0)
@@ -23744,7 +23752,7 @@ virDomainDefAddImplicitVideo(virDomainDefPtr def)
}
int
-virDomainDefAddImplicitDevices(virDomainDefPtr def)
+virDomainDefAddImplicitDevices(virDomainDefPtr def, virDomainXMLOptionPtr
xmlopt)
{
if (virDomainDefAddConsoleCompat(def) < 0)
return -1;
@@ -23752,7 +23760,7 @@ virDomainDefAddImplicitDevices(virDomainDefPtr def)
if (virDomainDefAddImplicitControllers(def) < 0)
return -1;
-if (virDomainDefAddImplicitVideo(def) < 0)
+if (virDomainDefAddImplicitVideo(def, xmlopt) < 0)
return -1;
return 0;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index faeb060c04..d8d60f8ca1 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1425,6 +1425,8 @@ struct _virDomainVideoDriverDef {
};
struct _virDomainVideoDef {
+virObjectPtr privateData;
+
int type; /* enum virDomainVideoType */
unsigned int ram; /* kibibytes (multiples of 1024) */
unsigned int vram; /* kibibytes (multiples of 1024) */
@@ -2726,6 +2728,7 @@ struct _virDomainXMLPrivateDataCallbacks {
virDomainXMLPrivateDataNewFuncchrSourceNew;
virDomainXMLPrivateDataNewFuncvsockNew;
virDomainXMLPrivateDataNewFuncgraphicsNew;
+virDomainXMLPrivateDataNewFuncvideoNew;
virDomainXMLPrivateDataFormatFunc format;
virDomainXMLPrivateDataParseFunc parse;
/* following function shall return a pointer which will be used as the
@@ -2867,7 +2870,7 @@ void virDomainSoundDefFree(virDomainSoundDefPtr def);
void virDomainMemballoonDefFree(virDomainMemballoonDefPtr def);
void virDomainNVRAMDefFree(virDomainNVRAMDefPtr def);
void virDomainWatchdogDefFree(virDomainWatchdogDefPtr def);
-virDomainVideoDefPtr virDomainVideoDefNew(void);
+virDomainVideoDefPtr virDomainVideoDefNew(virDomainXMLOptionPtr xmlopt);
void virDomainVideoDefFree(virDomainVideoDefPtr def);
void virD
[libvirt] [PATCH v3 13/19] qemu: add qemuDomainVideoPrivate
From: Marc-André Lureau
Signed-off-by: Marc-André Lureau
---
src/qemu/qemu_domain.c | 36
src/qemu/qemu_domain.h | 14 +-
2 files changed, 49 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f694cf8bbc..c0e4992f24 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1266,6 +1266,41 @@ qemuDomainGraphicsPrivateDispose(void *obj)
}
+static virClassPtr qemuDomainVideoPrivateClass;
+static void qemuDomainVideoPrivateDispose(void *obj);
+
+static int
+qemuDomainVideoPrivateOnceInit(void)
+{
+if (!VIR_CLASS_NEW(qemuDomainVideoPrivate, virClassForObject()))
+return -1;
+
+return 0;
+}
+
+VIR_ONCE_GLOBAL_INIT(qemuDomainVideoPrivate);
+
+static virObjectPtr
+qemuDomainVideoPrivateNew(void)
+{
+qemuDomainVideoPrivatePtr priv;
+
+if (qemuDomainVideoPrivateInitialize() < 0)
+return NULL;
+
+if (!(priv = virObjectNew(qemuDomainVideoPrivateClass)))
+return NULL;
+
+return (virObjectPtr) priv;
+}
+
+
+static void
+qemuDomainVideoPrivateDispose(void *obj ATTRIBUTE_UNUSED)
+{
+}
+
+
/* qemuDomainSecretPlainSetup:
* @secinfo: Pointer to secret info
* @usageType: The virSecretUsageType
@@ -3461,6 +3496,7 @@ virDomainXMLPrivateDataCallbacks
virQEMUDriverPrivateDataCallbacks = {
.chrSourceNew = qemuDomainChrSourcePrivateNew,
.vsockNew = qemuDomainVsockPrivateNew,
.graphicsNew = qemuDomainGraphicsPrivateNew,
+.videoNew = qemuDomainVideoPrivateNew,
.parse = qemuDomainObjPrivateXMLParse,
.format = qemuDomainObjPrivateXMLFormat,
.getParseOpaque = qemuDomainObjPrivateXMLGetParseOpaque,
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index d097f23342..1b51b27134 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -497,7 +497,19 @@ struct _qemuDomainVsockPrivate {
};
-#define QEMU_DOMAIN_GRAPHICS_PRIVATE(dev) \
+#define QEMU_DOMAIN_VIDEO_PRIVATE(dev) \
+((qemuDomainVideoPrivatePtr) (dev)->privateData)
+
+typedef struct _qemuDomainVideoPrivate qemuDomainVideoPrivate;
+typedef qemuDomainVideoPrivate *qemuDomainVideoPrivatePtr;
+struct _qemuDomainVideoPrivate {
+virObject parent;
+
+bool tmp_to_remove;
+};
+
+
+#define QEMU_DOMAIN_GRAPHICS_PRIVATE(dev) \
((qemuDomainGraphicsPrivatePtr) (dev)->privateData)
typedef struct _qemuDomainGraphicsPrivate qemuDomainGraphicsPrivate;
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 15/19] qemu: prepare domain for vhost-user GPU
From: Marc-André Lureau
Call qemuExtVhostUserGPUPrepareDomain() to fill the domain with the
location of the vhost-user binary to start.
Signed-off-by: Marc-André Lureau
---
src/qemu/qemu_extdevice.c | 29 +
src/qemu/qemu_extdevice.h | 5 +
src/qemu/qemu_process.c | 4
3 files changed, 38 insertions(+)
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
index dc032aa60c..b2d3cd3bd8 100644
--- a/src/qemu/qemu_extdevice.c
+++ b/src/qemu/qemu_extdevice.c
@@ -21,6 +21,7 @@
#include
#include "qemu_extdevice.h"
+#include "qemu_vhost_user_gpu.h"
#include "qemu_domain.h"
#include "qemu_tpm.h"
@@ -92,6 +93,34 @@ qemuExtDevicesInitPaths(virQEMUDriverPtr driver,
}
+/*
+ * qemuExtDevicesPrepareDomain:
+ *
+ * @driver: QEMU driver
+ * @vm: domain
+ *
+ * Code that modifies live XML of a domain which is about to start.
+ */
+int
+qemuExtDevicesPrepareDomain(virQEMUDriverPtr driver,
+virDomainObjPtr vm)
+{
+int ret = 0;
+size_t i;
+
+for (i = 0; i < vm->def->nvideos; i++) {
+virDomainVideoDefPtr video = vm->def->videos[i];
+
+if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) {
+if ((ret = qemuExtVhostUserGPUPrepareDomain(driver, video)) < 0)
+break;
+}
+}
+
+return ret;
+}
+
+
/*
* qemuExtDevicesPrepareHost:
*
diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h
index 039b3e60dd..2412244d60 100644
--- a/src/qemu/qemu_extdevice.h
+++ b/src/qemu/qemu_extdevice.h
@@ -29,6 +29,11 @@ int qemuExtDeviceLogCommand(qemuDomainLogContextPtr logCtxt,
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
ATTRIBUTE_RETURN_CHECK;
+int qemuExtDevicesPrepareDomain(virQEMUDriverPtr driver,
+virDomainObjPtr vm)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
+ATTRIBUTE_RETURN_CHECK;
+
int qemuExtDevicesPrepareHost(virQEMUDriverPtr driver,
virDomainDefPtr def)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 3fe46a7c7b..f2befe2111 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6357,6 +6357,10 @@ qemuProcessPrepareDomain(virQEMUDriverPtr driver,
if (qemuFirmwareFillDomain(driver, vm, flags) < 0)
goto cleanup;
+VIR_DEBUG("Preparing external devices");
+if (qemuExtDevicesPrepareDomain(driver, vm) < 0)
+goto cleanup;
+
for (i = 0; i < vm->def->nchannels; i++) {
if (qemuDomainPrepareChannel(vm->def->channels[i],
priv->channelTargetDir) < 0)
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 16/19] qemu: start/stop the vhost-user-gpu external device
From: Marc-André Lureau
Each vhost-user-gpu needs its own helper gpu process.
Start/stop them, and apply the emulator cgroup controller.
Signed-off-by: Marc-André Lureau
---
src/qemu/qemu_extdevice.c | 44 +++
1 file changed, 40 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
index b2d3cd3bd8..2cf311dd64 100644
--- a/src/qemu/qemu_extdevice.c
+++ b/src/qemu/qemu_extdevice.c
@@ -161,10 +161,21 @@ qemuExtDevicesStart(virQEMUDriverPtr driver,
bool incomingMigration)
{
int ret = 0;
+size_t i;
if (qemuExtDevicesInitPaths(driver, vm->def) < 0)
return -1;
+for (i = 0; i < vm->def->nvideos; i++) {
+virDomainVideoDefPtr video = vm->def->videos[i];
+
+if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) {
+ret = qemuExtVhostUserGPUStart(driver, vm, video, logCtxt);
+if (ret < 0)
+return ret;
+}
+}
+
if (vm->def->tpm)
ret = qemuExtTPMStart(driver, vm, logCtxt, incomingMigration);
@@ -176,9 +187,18 @@ void
qemuExtDevicesStop(virQEMUDriverPtr driver,
virDomainObjPtr vm)
{
+size_t i;
+
if (qemuExtDevicesInitPaths(driver, vm->def) < 0)
return;
+for (i = 0; i < vm->def->nvideos; i++) {
+virDomainVideoDefPtr video = vm->def->videos[i];
+
+if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER)
+qemuExtVhostUserGPUStop(driver, vm, video);
+}
+
if (vm->def->tpm)
qemuExtTPMStop(driver, vm);
}
@@ -187,6 +207,13 @@ qemuExtDevicesStop(virQEMUDriverPtr driver,
bool
qemuExtDevicesHasDevice(virDomainDefPtr def)
{
+size_t i;
+
+for (i = 0; i < def->nvideos; i++) {
+if (def->videos[i]->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER)
+return true;
+}
+
if (def->tpm && def->tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR)
return true;
@@ -199,10 +226,19 @@ qemuExtDevicesSetupCgroup(virQEMUDriverPtr driver,
virDomainDefPtr def,
virCgroupPtr cgroup)
{
-int ret = 0;
+size_t i;
-if (def->tpm)
-ret = qemuExtTPMSetupCgroup(driver, def, cgroup);
+for (i = 0; i < def->nvideos; i++) {
+virDomainVideoDefPtr video = def->videos[i];
-return ret;
+if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER &&
+qemuExtVhostUserGPUSetupCgroup(driver, def, video, cgroup) < 0)
+return -1;
+}
+
+if (def->tpm &&
+qemuExtTPMSetupCgroup(driver, def, cgroup) < 0)
+return -1;
+
+return 0;
}
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 14/19] qemu: add vhost-user-gpu helper unit
From: Marc-André Lureau
Similar to the qemu_tpm.c, add a unit with a few functions to
start/stop and setup the cgroup of the external vhost-user-gpu
process. See function documentation.
The vhost-user connection fd is set on qemuDomainVideoPrivate struct.
Signed-off-by: Marc-André Lureau
---
src/qemu/Makefile.inc.am | 2 +
src/qemu/qemu_domain.c | 5 +-
src/qemu/qemu_domain.h | 2 +-
src/qemu/qemu_vhost_user_gpu.c | 285 +
src/qemu/qemu_vhost_user_gpu.h | 50 ++
5 files changed, 342 insertions(+), 2 deletions(-)
create mode 100644 src/qemu/qemu_vhost_user_gpu.c
create mode 100644 src/qemu/qemu_vhost_user_gpu.h
diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
index aa22c1d2a7..ab5507be83 100644
--- a/src/qemu/Makefile.inc.am
+++ b/src/qemu/Makefile.inc.am
@@ -62,6 +62,8 @@ QEMU_DRIVER_SOURCES = \
qemu/qemu_tpm.h \
qemu/qemu_vhost_user.c \
qemu/qemu_vhost_user.h \
+ qemu/qemu_vhost_user_gpu.c \
+ qemu/qemu_vhost_user_gpu.h \
$(NULL)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c0e4992f24..1d78b12302 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1296,8 +1296,11 @@ qemuDomainVideoPrivateNew(void)
static void
-qemuDomainVideoPrivateDispose(void *obj ATTRIBUTE_UNUSED)
+qemuDomainVideoPrivateDispose(void *obj)
{
+qemuDomainVideoPrivatePtr priv = obj;
+
+VIR_FORCE_CLOSE(priv->vhost_user_fd);
}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 1b51b27134..9ed3046557 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -505,7 +505,7 @@ typedef qemuDomainVideoPrivate *qemuDomainVideoPrivatePtr;
struct _qemuDomainVideoPrivate {
virObject parent;
-bool tmp_to_remove;
+int vhost_user_fd;
};
diff --git a/src/qemu/qemu_vhost_user_gpu.c b/src/qemu/qemu_vhost_user_gpu.c
new file mode 100644
index 00..a77805c819
--- /dev/null
+++ b/src/qemu/qemu_vhost_user_gpu.c
@@ -0,0 +1,285 @@
+/*
+ * qemu_vhost_user_gpu.c: QEMU vhost-user GPU support
+ *
+ * Copyright (C) 2019 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ *
[libvirt] [PATCH v3 18/19] tests: mock execv/execve
From: Marc-André Lureau
Learn to override the paths to the program to execute (vhost-user
helpers are executed to check for runtime capabilities).
Signed-off-by: Marc-André Lureau
---
tests/virfilewrapper.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/tests/virfilewrapper.c b/tests/virfilewrapper.c
index 160cd571e0..3d3f319f2c 100644
--- a/tests/virfilewrapper.c
+++ b/tests/virfilewrapper.c
@@ -44,6 +44,8 @@ static FILE *(*real_fopen)(const char *path, const char
*mode);
static int (*real_access)(const char *path, int mode);
static int (*real_mkdir)(const char *path, mode_t mode);
static DIR *(*real_opendir)(const char *path);
+static int (*real_execv)(const char *path, char *const argv[]);
+static int (*real_execve)(const char *path, char *const argv[], char *const
envp[]);
static void init_syms(void)
{
@@ -55,6 +57,8 @@ static void init_syms(void)
VIR_MOCK_REAL_INIT(mkdir);
VIR_MOCK_REAL_INIT(open);
VIR_MOCK_REAL_INIT(opendir);
+VIR_MOCK_REAL_INIT(execv);
+VIR_MOCK_REAL_INIT(execve);
}
@@ -191,4 +195,22 @@ DIR *opendir(const char *path)
return real_opendir(newpath ? newpath : path);
}
+int execv(const char *path, char *const argv[])
+{
+VIR_AUTOFREE(char *) newpath = NULL;
+
+PATH_OVERRIDE(newpath, path);
+
+return real_execv(newpath ? newpath : path, argv);
+}
+
+int execve(const char *path, char *const argv[], char *const envp[])
+{
+VIR_AUTOFREE(char *) newpath = NULL;
+
+PATH_OVERRIDE(newpath, path);
+
+return real_execve(newpath ? newpath : path, argv, envp);
+}
+
#endif
--
2.23.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 17/19] qemu: build vhost-user GPU devices
From: Marc-André Lureau
For each vhost-user GPUs,
- build a socket chardev, and pass the vhost-user socket to it
- build a vhost-user video device and associate it with the chardev
Signed-off-by: Marc-André Lureau
---
src/qemu/qemu_command.c| 56 +++---
tests/qemuxml2argvdata/virtio-options.args | 5 +-
2 files changed, 52 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index e232406ff9..bf71f447a4 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4519,16 +4519,23 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
virQEMUCapsPtr qemuCaps)
{
VIR_AUTOCLEAN(virBuffer) buf = VIR_BUFFER_INITIALIZER;
-const char *model;
+const char *model = NULL;
/* We try to chose the best model for primary video device by preferring
* model with VGA compatibility mode. For some video devices on some
* architectures there might not be such model so fallback to one
* without VGA compatibility mode. */
-if (video->primary && qemuDomainSupportsVideoVga(video, qemuCaps))
-model = qemuDeviceVideoTypeToString(video->type);
-else
-model = qemuDeviceVideoSecondaryTypeToString(video->type);
+if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) {
+if (video->primary && qemuDomainSupportsVideoVga(video, qemuCaps))
+model = "vhost-user-vga";
+else
+model = "vhost-user-gpu";
+} else {
+if (video->primary && qemuDomainSupportsVideoVga(video, qemuCaps))
+model = qemuDeviceVideoTypeToString(video->type);
+else
+model = qemuDeviceVideoSecondaryTypeToString(video->type);
+}
if (!model || STREQ(model, "")) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -4537,8 +4544,8 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
return NULL;
}
-if (STREQ(model, "virtio-gpu")) {
-if (qemuBuildVirtioDevStr(&buf, "virtio-gpu", qemuCaps,
+if (STREQ(model, "virtio-gpu") || STREQ(model, "vhost-user-gpu")) {
+if (qemuBuildVirtioDevStr(&buf, model, qemuCaps,
VIR_DOMAIN_DEVICE_VIDEO, video) < 0) {
return NULL;
}
@@ -4581,6 +4588,10 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
if (video->heads)
virBufferAsprintf(&buf, ",max_outputs=%u", video->heads);
}
+} else if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) {
+if (video->heads)
+virBufferAsprintf(&buf, ",max_outputs=%u", video->heads);
+virBufferAsprintf(&buf, ",chardev=chr-vu-%s", video->info.alias);
} else if (video->type == VIR_DOMAIN_VIDEO_TYPE_VIRTIO) {
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_GPU_MAX_OUTPUTS)) {
if (video->heads)
@@ -4692,6 +4703,23 @@ qemuBuildVgaVideoCommand(virCommandPtr cmd,
}
+static char *
+qemuBuildVhostUserChardevStr(const char *alias,
+ int *fd,
+ virCommandPtr cmd)
+{
+char *chardev = NULL;
+
+if (virAsprintf(&chardev, "socket,id=chr-vu-%s,fd=%d", alias, *fd) < 0)
+return NULL;
+
+virCommandPassFD(cmd, *fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
+*fd = -1;
+
+return chardev;
+}
+
+
static int
qemuBuildVideoCommandLine(virCommandPtr cmd,
const virDomainDef *def,
@@ -4699,6 +4727,20 @@ qemuBuildVideoCommandLine(virCommandPtr cmd,
{
size_t i;
+for (i = 0; i < def->nvideos; i++) {
+VIR_AUTOFREE(char *) chardev = NULL;
+virDomainVideoDefPtr video = def->videos[i];
+
+if (video->backend == VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER) {
+if (!(chardev = qemuBuildVhostUserChardevStr(video->info.alias,
+
&QEMU_DOMAIN_VIDEO_PRIVATE(video)->vhost_user_fd,
+cmd)))
+return -1;
+
+virCommandAddArgList(cmd, "-chardev", chardev, NULL);
+}
+}
+
for (i = 0; i < def->nvideos; i++) {
VIR_AUTOFREE(char *) str = NULL;
virDomainVideoDefPtr video = def->videos[i];
diff --git a/tests/qemuxml2argvdata/virtio-options.args
b/tests/qemuxml2argvdata/virtio-options.args
index 69dc1c73e5..f3d0f507f6 100644
--- a/tests/qemuxml2argvdata/virtio-options.args
+++ b/tests/qemuxml2argvdata/virtio-options.args
@@ -49,8 +49,9 @@ ats=on \
ats=on \
-device virtio-input-host-pci,id=input3,evdev=/dev/input/event1234,bus=pci.0,\
addr=0x12,iommu_platform=on,ats=on \
--device virtio-gpu-pci,id=video0,bus=pci.0,addr=0x2,iommu_platform=on,\
-ats=on \
+-chardev socket,id=chr-vu-video0,fd=0 \
+-device vhost-user-gpu-pci,id=video0,max_outputs=1,chardev=chr-vu-video0\
+,bus=pci.0,addr=0x2,iommu_platform=on,ats=on \
-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0xc,iommu_platform=on,\
ats=on \
-object rng-random,id=o
[libvirt] [PATCH v3 19/19] tests: add vhost-user-gpu xml2argv tests
From: Marc-André Lureau Signed-off-by: Marc-André Lureau --- ...host-user-gpu-secondary.x86_64-latest.args | 43 + .../vhost-user-gpu-secondary.xml | 46 +++ .../vhost-user-vga.x86_64-latest.args | 40 tests/qemuxml2argvdata/vhost-user-vga.xml | 42 + tests/qemuxml2argvtest.c | 12 + 5 files changed, 183 insertions(+) create mode 100644 tests/qemuxml2argvdata/vhost-user-gpu-secondary.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/vhost-user-gpu-secondary.xml create mode 100644 tests/qemuxml2argvdata/vhost-user-vga.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/vhost-user-vga.xml diff --git a/tests/qemuxml2argvdata/vhost-user-gpu-secondary.x86_64-latest.args b/tests/qemuxml2argvdata/vhost-user-gpu-secondary.x86_64-latest.args new file mode 100644 index 00..58f49595e7 --- /dev/null +++ b/tests/qemuxml2argvdata/vhost-user-gpu-secondary.x86_64-latest.args @@ -0,0 +1,43 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-object memory-backend-memfd,id=ram-node0,share=yes,size=224395264 \ +-numa node,nodeid=0,cpus=0,memdev=ram-node0 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ +-chardev socket,id=chr-vu-video0,fd=0 \ +-chardev socket,id=chr-vu-video1,fd=0 \ +-device vhost-user-vga,id=video0,max_outputs=1,chardev=chr-vu-video0,bus=pci.0,\ +addr=0x2 \ +-device vhost-user-gpu-pci,id=video1,max_outputs=1,chardev=chr-vu-video1,\ +bus=pci.0,addr=0x4 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvdata/vhost-user-gpu-secondary.xml b/tests/qemuxml2argvdata/vhost-user-gpu-secondary.xml new file mode 100644 index 00..bb1f345532 --- /dev/null +++ b/tests/qemuxml2argvdata/vhost-user-gpu-secondary.xml @@ -0,0 +1,46 @@ + + QEMUGuest1 + c7a5fdbd-edaf-9455-926a-d65c16db1809 + 219136 + 219136 + + + + + 1 + + + + + + +hvm + + + + destroy + restart + destroy + +/usr/bin/qemu-system-i686 + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/qemuxml2argvdata/vhost-user-vga.x86_64-latest.args b/tests/qemuxml2argvdata/vhost-user-vga.x86_64-latest.args new file mode 100644 index 00..6640d86fa5 --- /dev/null +++ b/tests/qemuxml2argvdata/vhost-user-vga.x86_64-latest.args @@ -0,0 +1,40 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-QEMUGuest1 \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-i686 \ +-name guest=QEMUGuest1,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \ +-machine pc,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-object memory-backend-memfd,id=ram-node0,share=yes,size=224395264 \ +-numa node,nodeid=0,cpus=0,memdev=ram-node0 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-no-acpi \ +-boot strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \ +-device ide-hd,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0,bootindex=1 \ +-chardev socket,id=chr-vu-video0,fd=0 \ +-device vhost-user-vga,id=video0,max_outputs=1,chardev=chr-vu-video0,bus=pci.0,\ +addr=0x2 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \ +-sandbox on,obsolete=deny,elevateprivileges
Re: [libvirt] [Qemu-devel] [PATCH 2/2] qapi: deprecate implicit filters
On 8/30/19 6:07 AM, Christophe de Dinechin wrote:
>
> John Snow writes:
>
>> On 8/29/19 12:45 PM, Christophe de Dinechin wrote:
>>>
> [...]
>
>>> Sorry for catching up late, this mail thread happened during my PTO.
>>>
>>> I remember bringing up at the time [1] that the correct solution needs
>>> to take into account usage models that vary from
>>>
>>> - a workstation case, where displaying an error box is easy and
>>> convenient,
>>>
>>> - to local headless VMs where system-level notification would do the job
>>> better, allowing us to leverage things like system-wide email
>>> notifications
>>>
>>> - to large-scale collections of VMs managed by some layered product,
>>> where the correct reporting would be through something like Insights,
>>> i.e. you don't scan individual logs, you want something like "913 VMs
>>> are using deprecated X"
>>>
>>> To me, that implies that we need to have a clear division of roles, with
>>> a standard way to
>>>
>>> a) produce the errors,
>>> b) propagate them,
>>
>> I started replying to this thread to the other mail you sent; I think
>> this is going to be fairly involved. I wouldn't mind being proven wrong
>> though.
>
> Yes, I think it does look involved, but mostly for historical reasons.
> In other words, what is complicated is preserving the historical
> behaviors so as to not break existing consumers.
>
>>
>>> c) consume them (at least up to libvirt)
>>>
>>> Notice that this work has already been done for "real" errors,
>>> i.e. there is a real QAPI notion of "errors". AFAICT, warn_report does
>>> not connect to it, though, it goes through error_vprintf which is really
>>> just basic logging.
>>>
>>> So would it make sense to:
>>>
>>> 1. Add a deprecation_report() alongside warn_report()?
>>>
>>
>> Where's that get routed to? just an error_vprintf style situation?
>
> Yes, but see below.
>
>>
>>> 2. Connect warn_report() and all the error_vprintf output to QAPI,
>>>e.g. using John's suggestion of adding the messages using some
>>>"warning" or "deprecated" tag?
>>>
>>
>> How do you correlate them?
>
> Without having looked at the code much, I think I would
>
> 1. extend the existing QAPI error to support warnings, deprecations and
>info messages. The first problem I see is that there is no error, so
>we may sometimes need to create one when there was none before. And
>of course make sure that this does not ultimately show as an error,
>but as a success with additional annotations.
>
I assume this might be a chance to consolidate all of the methodologies
we use for actually checking if there was an error or not. There have
been many and I am sure Markus can give us a history lesson if it's
warranted.
Generally, there's a few paradigms I see a lot:
1. Rely on an error return code being produced by the called function.
The caller trusts that errp was set. This is one of my favorite methods,
because it has the least scaffolding.
2. Pass errp directly to the called function, and check for null after
return. I don't like this method very much, because of confusion with:
3. Create a local error object; check THAT for null, and propagate the
error to the common error object. I think Markus has explained why we
have this code 50 times, and I forget again minutes later.
If we want to expand the concept of the error object into something that
encompasses hints, warnings and deprecations*, checking for null is no
longer appropriate. It might be a good chance to make our error
propagation story more consistent, too.
We could unify with a helper like this, I think, if I'm not forgetting
some crucial usage detail:
subroutine(foo, bar, errp);
if (failure(errp)) {
error_append_hint(errp, "Lorem ipsum, ...");
cleanup();
return;
}
We would then always use this pattern that operates directly on the
caller's errp instead of creating local error objects to allow hints and
warnings to accumulate.
(* I'm not proposing all three in a concrete way, but just referencing
the general class of semantic non-error information we may want to
propagate, however we end up deciding to model it.)
> 2. replace the current "link + if" switching for error_vprintf with some
>actual notification mechanism, with one option routine to
>monitor_vprintf, one to stderr, one to log file, and then an
>additional path that would post a newly minted qapi warning.
>
>>
>>> 3. Teach libvirt how to consume that new tag and pass it along?
>>>
>>
>> I think it's not libvirt's job to pass it along, exactly -- libvirt made
>> the decision for which features to engage in QEMU, not the end user.
>
> First, by "pass along", I meant to possible layered products or
> management software. We don't necessarily need a new virErrorLevel,
> deprecation could be a warning with some special domain,
> e.g. VIR_FROM_DEPRECATION.
>
SHOULD it pass it along? Libvirt knows what QMP is invoking, that should
be opaque to upper layers. a "DEPRECATION" log
Re: [libvirt] [BUG] When PLUG a bridge interface to an active VM, the generated LIVE and CONFIG mac address are different
On Fri, Aug 30, 2019 at 10:14:17AM +0200, Michal Privoznik wrote:
On 8/30/19 8:51 AM, wujing (O) wrote:
On 8/28/19 11:42 AM, wujing (O) wrote:
On 8/22/19 10:43 AM, Xu Yandong (Yandong Xu) wrote:
That possibly might be out of scope, but autofilling the mac
address as early as virDomainNetDefParseXML also is not ideal.
We have pushed a patch bellow that can restore the situation to an
older
state.
Subject: [PATCH] qemu: use the same def when attaching device live
and config
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index
617d7d5..eca54d0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8615,6 +8615,22 @@
qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
goto cleanup;
+if (flags & VIR_DOMAIN_AFFECT_CONFIG &&
+flags & VIR_DOMAIN_AFFECT_LIVE) {
+/* If we are affecting both CONFIG and LIVE
+ * use the same xml of device preferentially
+ * to make the configuration consistent.
+ */
+devLive = virDomainDeviceDefParse(xml, vm->def,
This use of vm->def prefers live XML. This means that for instance
PCI address are assigned based on current PCI layout in live XML
which in general is different to inactive XML.
And it's not only PCI addresses, we autogenerate some out aspects of
(e.g. model) and even more for other devices. Just take
a look at qemuDomainDeviceDefPostParse(). All functions there which
take domain def as an argument do so because they are basing
autogenerated value on domain definition. Plus there's more in parser
code.
While your patch might work for your use case, it can break others.
BTW: have you read the original commit that caused this? I'm failing
to see how we would not re-introduce the problem with your patch. If
you're using live XML to validate/generate device addresses, how can
we generate valid address for inactive XML?
This patch is a little different with the original commit, since we
only prefer live XML when using LIVE AND CONFIG flags at the same time.
Still parse separate device def if attach with only one flag.
So this patch is kinda a combination of 1e0534a7702 and 55ce6564634, just
to narrow down the problem influence.
And I verified as the steps in
https://bugzilla.redhat.com/show_bug.cgi?id=1559867, work as expected.
Well, then try to coldplug (that is
only to inactive XML), and then try hotplug different to
both live and inactive XMLs and the bug will reproduce.
Problem is, that inactive XML contains a device which is not contained in live
XML and thus when generating device address based solely on live XML a
conflicting address is generated.
If we'd prefer inactive XML in your patch then the steps in my reproducer
need to be swapped. At any rate, the issue is still there.
You are right, but I found a new issue on current solution.
Test as steps below.
1. Having a running vm
## virsh list
IdName State
-- │~
1rhel running
2. prepare 2 scsi hostdev xml using the same address
## cat h1.xml
## cat h2.xml
3. attach 2 hostdev to vm with 'attach-device --config'
## virsh attach-device rhel h1.xml --config
Device attached successfully
## virsh attach-device rhel h2.xml --config
Device attached successfully
4. Check the vm's xml with 'dumpxml --inactive'
## virsh dumpxml rhel --inactive
...
...
a) Looks like libvirt can not verify if user configured the same address for
different scsi hostdevs.
Because virDomainHostdevDefPostParse only calls
virDomainDriveAddressIsUsedByDisk to check
if address is used by other disks, but no check if used by other hostdevs.
And qemuDomainAttachDeviceConfig calls virDomainHostdevFind to check duplicate
devices by
src addr and other stuff, not including addr in vm.
Should that be fixed?
Oh yeah, this is a long known issue:
https://bugzilla.redhat.com/show_bug.cgi?id=1729022
and I want to fix it, but I always get side tracked :-( So if somebody
beats me to it, I wouldn't mind ;-)
b) If we fixed a), then we can get a devConf that both fits to live and config
xml by
passing devLive and domainPersistentDef to virDomainDeviceDefCopy.
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 617d7d5..eca54d0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8615,6 +8615,22 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
goto cleanup;
+if (flags & VIR_DOMAIN_AFFECT_CONFIG &&
+flags & VIR_DOMAIN_AFFECT_LIVE) {
+devLive = virDomainDeviceDefParse(xml, vm->def,
+ caps, driver->xmlopt,
+
Re: [libvirt] [PATCH] qemu: Validate arg in qemuAgentErrorComandUnsupported()
On 8/30/19 12:09 PM, Jonathon Jongsma wrote: > Coverity noted that 'reply' can be NULL after calling > qemuAgentCommand(). Avoid dereferencing reply in > qemuAgentErrorComandUnsupported() in that case. > > Signed-off-by: Jonathon Jongsma > --- > src/qemu/qemu_agent.c | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > Reviewed-by: John Ferlan John (I've pushed as a bugfix during freeze) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 00/10] Adding resolution properties for video models
From: Julio Faracco
This serie adds 'xres' and 'yres' QEMU display properties into a new
element called 'resolution'. This element is covered by model element:
Only types VGA, QXL, Virtio and Bochs support passing resolution to
driver. That's why some test cases were added too.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1485793
Julio Faracco (10):
docs: Adding 'xres' and 'yres' into qxl XML definition
qemu: Include {xres,yres} QEMU capabilities for video models
conf: Adding resolution property for model element
qemu: Include {xres,yres} for QEMU command line
tests: Include {xres,yres} QEMU capabilities into tests
tests: Include bochs-display as capability test too
tests: Introduce resolution test for QXL model
tests: Introduce resolution test for Virtio model
tests: Introduce resolution test for Bochs model
tests: Introduce resolution test for VGA model
docs/schemas/domaincommon.rng | 10 ++
src/conf/domain_conf.c| 75 +++-
src/conf/domain_conf.h| 5 +
src/conf/virconftypes.h | 3 +
src/qemu/qemu_capabilities.c | 16 ++
src/qemu/qemu_capabilities.h | 2 +
src/qemu/qemu_command.c | 20 +++
.../caps_2.10.0.aarch64.xml | 2 +
.../caps_2.10.0.ppc64.xml | 2 +
.../caps_2.10.0.s390x.xml | 2 +
.../caps_2.10.0.x86_64.xml| 2 +
.../caps_2.11.0.s390x.xml | 2 +
.../caps_2.11.0.x86_64.xml| 2 +
.../caps_2.12.0.aarch64.xml | 2 +
.../caps_2.12.0.ppc64.xml | 2 +
.../caps_2.12.0.s390x.xml | 2 +
.../caps_2.12.0.x86_64.xml| 2 +
.../caps_3.0.0.ppc64.replies | 139 +++
.../qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 2 +
.../caps_3.0.0.riscv32.xml| 2 +
.../caps_3.0.0.riscv64.xml| 2 +
.../qemucapabilitiesdata/caps_3.0.0.s390x.xml | 2 +
.../caps_3.0.0.x86_64.replies | 167 +-
.../caps_3.0.0.x86_64.xml | 2 +
.../caps_3.1.0.ppc64.replies | 139 +++
.../qemucapabilitiesdata/caps_3.1.0.ppc64.xml | 2 +
.../caps_3.1.0.x86_64.replies | 167 +-
.../caps_3.1.0.x86_64.xml | 2 +
.../caps_4.0.0.aarch64.replies| 139 +++
.../caps_4.0.0.aarch64.xml| 2 +
.../caps_4.0.0.ppc64.replies | 139 +++
.../qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 2 +
.../caps_4.0.0.riscv32.replies| 131 +++---
.../caps_4.0.0.riscv32.xml| 2 +
.../caps_4.0.0.riscv64.replies| 131 +++---
.../caps_4.0.0.riscv64.xml| 2 +
.../qemucapabilitiesdata/caps_4.0.0.s390x.xml | 2 +
.../caps_4.0.0.x86_64.replies | 167 +-
.../caps_4.0.0.x86_64.xml | 2 +
.../caps_4.1.0.x86_64.replies | 159 -
.../caps_4.1.0.x86_64.xml | 2 +
...ochs-display-resolution.x86_64-latest.args | 35
.../video-bochs-display-resolution.xml| 33
.../video-qxl-resolution.x86_64-latest.args | 35
.../qemuxml2argvdata/video-qxl-resolution.xml | 33
...o-virtio-gpu-resolution.x86_64-latest.args | 35
.../video-virtio-gpu-resolution.xml | 33
tests/qemuxml2argvtest.c | 4 +
...bochs-display-resolution.x86_64-latest.xml | 33
.../video-qxl-resolution.x86_64-latest.xml| 33
...eo-virtio-gpu-resolution.x86_64-latest.xml | 33
tests/qemuxml2xmltest.c | 5 +
52 files changed, 1604 insertions(+), 365 deletions(-)
create mode 100644
tests/qemuxml2argvdata/video-bochs-display-resolution.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/video-bochs-display-resolution.xml
create mode 100644
tests/qemuxml2argvdata/video-qxl-resolution.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/video-qxl-resolution.xml
create mode 100644
tests/qemuxml2argvdata/video-virtio-gpu-resolution.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-resolution.xml
create mode 100644
tests/qemuxml2xmloutdata/video-bochs-display-resolution.x86_64-latest.xml
create mode 100644
tests/qemuxml2xmloutdata/video-qxl-resolution.x86_64-latest.xml
create mode 100644
tests/qemuxml2xmloutdata/video-virtio-gpu-resolution.x86_64-latest.xml
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 05/10] tests: Include {xres, yres} QEMU capabilities into tests
From: Julio Faracco Both resolution capabilities were added for QEMU versions: 2.10, 2.11, 2.12, 3.0.0, 3.10 and 4.0.0. Signed-off-by: Julio Faracco --- tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml | 2 ++ tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml | 2 ++ tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 2 ++ tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml | 2 ++ tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 2 ++ tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml | 2 ++ tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 2 ++ tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 2 ++ tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 2 ++ tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 2 ++ tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml| 2 ++ tests/qemucapabilitiesdata/caps_3.0.0.riscv32.xml | 2 ++ tests/qemucapabilitiesdata/caps_3.0.0.riscv64.xml | 2 ++ tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml| 2 ++ tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml | 2 ++ tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml| 2 ++ tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml | 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml | 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml| 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml | 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml | 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml| 2 ++ tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml | 2 ++ tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml | 2 ++ 24 files changed, 48 insertions(+) diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml index 9404e66144..0dc2268068 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.aarch64.xml @@ -139,6 +139,8 @@ + + 201 0 61700805 diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml index 162fb1f035..29def35d43 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.ppc64.xml @@ -138,6 +138,8 @@ + + 201 0 42900805 diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml index 21b918f8d4..e4c0afb6e6 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml @@ -103,6 +103,8 @@ + + 201 0 39100805 diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml index 2788fd9afe..d4b7a88c8a 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.x86_64.xml @@ -181,6 +181,8 @@ + + 201 0 43100805 diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml index 6cb997d299..e1dd982a3e 100644 --- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml @@ -110,6 +110,8 @@ + + 2011000 0 39100806 diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml index 69f7fc2e4a..9e020f0e8e 100644 --- a/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml @@ -187,6 +187,8 @@ + + 2011000 0 43100806 diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml index 614fd14fb1..5b8469816b 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml @@ -153,6 +153,8 @@ + + 2012000 0 61700807 diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml index fd9ae0bcb8..e2b7a55e47 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml @@ -151,6 +151,8 @@ + + 2011090 0 42900807 diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml index 2930381068..b91c038ab8 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml @@ -121,6 +121,8 @@ + + 2012000 0 39100807 diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml index 61b3602c48..b6632a7553 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml @@ -195,6 +195,8 @@ + + 2011090 0 43100807 diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml b/tests/qemucapabil
[libvirt] [PATCH v2 08/10] tests: Introduce resolution test for Virtio model
From: Julio Faracco
New test case was added to cover 'xres' and 'yres' properties for Virtio
model due to latest QEMU version.
Signed-off-by: Julio Faracco
---
...o-virtio-gpu-resolution.x86_64-latest.args | 35 +++
.../video-virtio-gpu-resolution.xml | 33 +
tests/qemuxml2argvtest.c | 1 +
...eo-virtio-gpu-resolution.x86_64-latest.xml | 33 +
tests/qemuxml2xmltest.c | 1 +
5 files changed, 103 insertions(+)
create mode 100644
tests/qemuxml2argvdata/video-virtio-gpu-resolution.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/video-virtio-gpu-resolution.xml
create mode 100644
tests/qemuxml2xmloutdata/video-virtio-gpu-resolution.x86_64-latest.xml
diff --git
a/tests/qemuxml2argvdata/video-virtio-gpu-resolution.x86_64-latest.args
b/tests/qemuxml2argvdata/video-virtio-gpu-resolution.x86_64-latest.args
new file mode 100644
index 00..20702a4c81
--- /dev/null
+++ b/tests/qemuxml2argvdata/video-virtio-gpu-resolution.x86_64-latest.args
@@ -0,0 +1,35 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
+-machine pc,accel=tcg,usb=off,dump-guest-core=off \
+-m 1024 \
+-overcommit mem-lock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-boot strict=on \
+-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
+-device virtio-vga,id=video0,max_outputs=1,xres=1280,yres=720,\
+bus=pci.0,addr=0x2 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
+resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/video-virtio-gpu-resolution.xml
b/tests/qemuxml2argvdata/video-virtio-gpu-resolution.xml
new file mode 100644
index 00..2641a18711
--- /dev/null
+++ b/tests/qemuxml2argvdata/video-virtio-gpu-resolution.xml
@@ -0,0 +1,33 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 1048576
+ 1048576
+ 1
+
+hvm
+
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu-system-x86_64
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 1c3e3a8487..2e46dbd177 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2037,6 +2037,7 @@ mymain(void)
DO_TEST("video-virtio-gpu-secondary",
QEMU_CAPS_DEVICE_VIRTIO_GPU,
QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
+DO_TEST_CAPS_LATEST("video-virtio-gpu-resolution");
DO_TEST("video-virtio-vga",
QEMU_CAPS_DEVICE_VIRTIO_GPU,
QEMU_CAPS_DEVICE_VIRTIO_VGA,
diff --git
a/tests/qemuxml2xmloutdata/video-virtio-gpu-resolution.x86_64-latest.xml
b/tests/qemuxml2xmloutdata/video-virtio-gpu-resolution.x86_64-latest.xml
new file mode 100644
index 00..2641a18711
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/video-virtio-gpu-resolution.x86_64-latest.xml
@@ -0,0 +1,33 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 1048576
+ 1048576
+ 1
+
+hvm
+
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu-system-x86_64
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 9ea94815ae..9e6c4345f1 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1223,6 +1223,7 @@ mymain(void)
QEMU_CAPS_VIRTIO_GPU_MAX_OUTPUTS,
QEMU_CAPS_VNC,
QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW);
+DO_TEST_CAPS_LATEST("video-virtio-gpu-resolution");
DO_TEST("video-none-device", NONE);
DO_TEST_CAPS_LATEST("intel-iommu");
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 04/10] qemu: Include {xres, yres} for QEMU command line
From: Julio Faracco
Display devices can use now properties 'xres' and 'yres' by QEMU
command line. Only models specified by their QEMU capabilities. This
commit includes VGA, QXL, Virtio and Bochs.
Signed-off-by: Julio Faracco
---
src/qemu/qemu_command.c | 20
1 file changed, 20 insertions(+)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1ca1ecd2f0..566e34a2c7 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4564,6 +4564,11 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
virBufferAsprintf(&buf, ",vram_size=%u", video->vram * 1024);
}
+if (video->res && video->res->x && video->res->y) {
+/* QEMU accepts resolution xres and yres. */
+virBufferAsprintf(&buf, ",xres=%u,yres=%u", video->res->x,
video->res->y);
+}
+
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VRAM64)) {
/* QEMU accepts mebibytes for vram64_size_mb. */
virBufferAsprintf(&buf, ",vram64_size_mb=%u", video->vram64 /
1024);
@@ -4583,15 +4588,30 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
if (video->heads)
virBufferAsprintf(&buf, ",max_outputs=%u", video->heads);
}
+
+if (video->res && video->res->x && video->res->y) {
+/* QEMU accepts resolution xres and yres. */
+virBufferAsprintf(&buf, ",xres=%u,yres=%u", video->res->x,
video->res->y);
+}
} else if ((video->type == VIR_DOMAIN_VIDEO_TYPE_VGA &&
virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_VGAMEM)) ||
(video->type == VIR_DOMAIN_VIDEO_TYPE_VMVGA &&
virQEMUCapsGet(qemuCaps, QEMU_CAPS_VMWARE_SVGA_VGAMEM))) {
if (video->vram)
virBufferAsprintf(&buf, ",vgamem_mb=%u", video->vram / 1024);
+
+if (video->res && video->res->x && video->res->y) {
+/* QEMU accepts resolution xres and yres. */
+virBufferAsprintf(&buf, ",xres=%u,yres=%u", video->res->x,
video->res->y);
+}
} else if (video->type == VIR_DOMAIN_VIDEO_TYPE_BOCHS) {
if (video->vram)
virBufferAsprintf(&buf, ",vgamem=%uk", video->vram);
+
+if (video->res && video->res->x && video->res->y) {
+/* QEMU accepts resolution xres and yres. */
+virBufferAsprintf(&buf, ",xres=%u,yres=%u", video->res->x,
video->res->y);
+}
}
if (qemuBuildDeviceAddressStr(&buf, def, &video->info, qemuCaps) < 0)
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 10/10] tests: Introduce resolution test for VGA model
From: Julio Faracco
New test case was added to cover 'xres' and 'yres' properties for VGA
model due to latest QEMU version.
Signed-off-by: Julio Faracco
---
tests/qemuxml2argvtest.c | 1 +
tests/qemuxml2xmltest.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index f68c31d5e9..66b63925d2 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1987,6 +1987,7 @@ mymain(void)
QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
DO_TEST("video-vga-device-vgamem", QEMU_CAPS_DEVICE_VGA,
QEMU_CAPS_DEVICE_VIDEO_PRIMARY, QEMU_CAPS_VGA_VGAMEM);
+DO_TEST_CAPS_LATEST("video-vga-resolution");
DO_TEST("video-qxl-nodevice", QEMU_CAPS_DEVICE_QXL);
DO_TEST("video-qxl-device",
QEMU_CAPS_DEVICE_QXL,
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 3078355019..ac537767ba 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1200,6 +1200,7 @@ mymain(void)
DO_TEST("acpi-table", NONE);
+DO_TEST_CAPS_LATEST("video-vga-resolution");
DO_TEST("video-device-pciaddr-default",
QEMU_CAPS_KVM,
QEMU_CAPS_VNC,
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 03/10] conf: Adding resolution property for model element
From: Julio Faracco
New element 'resolution' with parameters 'x' and 'y' were added to
support this settings for VGA, QXL, Virtio and Bochs XMLs. A new
structure was created as Acceleration element has. It is easy to parse
this property. Example:
Signed-off-by: Julio Faracco
---
src/conf/domain_conf.c | 75 -
src/conf/domain_conf.h | 5 +++
src/conf/virconftypes.h | 3 ++
3 files changed, 82 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b7a342bb91..9db8fd9697 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15311,6 +15311,53 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
return def;
}
+static virDomainVideoResolutionDefPtr
+virDomainVideoResolutionDefParseXML(xmlNodePtr node)
+{
+xmlNodePtr cur;
+virDomainVideoResolutionDefPtr def;
+VIR_AUTOFREE(char *) x = NULL;
+VIR_AUTOFREE(char *) y = NULL;
+
+cur = node->children;
+while (cur != NULL) {
+if (cur->type == XML_ELEMENT_NODE) {
+if (!x && !y &&
+virXMLNodeNameEqual(cur, "resolution")) {
+x = virXMLPropString(cur, "x");
+y = virXMLPropString(cur, "y");
+}
+}
+cur = cur->next;
+}
+
+if (!x || !y)
+return NULL;
+
+if (VIR_ALLOC(def) < 0)
+goto cleanup;
+
+if (x) {
+if (virStrToLong_uip(x, NULL, 10, &def->x) < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("cannot parse video x-resolution '%s'"), x);
+goto cleanup;
+}
+}
+
+if (y) {
+if (virStrToLong_uip(y, NULL, 10, &def->y) < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("cannot parse video y-resolution '%s'"), y);
+goto cleanup;
+}
+}
+
+ cleanup:
+return def;
+}
+
+
static virDomainVideoDriverDefPtr
virDomainVideoDriverDefParseXML(xmlNodePtr node)
{
@@ -15389,6 +15436,7 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt,
}
def->accel = virDomainVideoAccelDefParseXML(cur);
+def->res = virDomainVideoResolutionDefParseXML(cur);
}
if (virXMLNodeNameEqual(cur, "driver")) {
if (virDomainVirtioOptionsParseXML(cur, &def->virtio) < 0)
@@ -15463,6 +15511,17 @@ virDomainVideoDefParseXML(virDomainXMLOptionPtr xmlopt,
}
}
+if (def->res) {
+if (def->type != VIR_DOMAIN_VIDEO_TYPE_VGA &&
+def->type != VIR_DOMAIN_VIDEO_TYPE_QXL &&
+def->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO &&
+def->type != VIR_DOMAIN_VIDEO_TYPE_BOCHS) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("model resolution is not supported"));
+goto error;
+}
+}
+
if (virDomainDeviceInfoParseXML(xmlopt, node, &def->info, flags) < 0)
goto error;
@@ -26443,6 +26502,18 @@ virDomainVideoAccelDefFormat(virBufferPtr buf,
virBufferAddLit(buf, "/>\n");
}
+static void
+virDomainVideoResolutionDefFormat(virBufferPtr buf,
+ virDomainVideoResolutionDefPtr def)
+{
+virBufferAddLit(buf, "x && def->y) {
+virBufferAsprintf(buf, " x='%u' y='%u'",
+ def->x, def->y);
+}
+virBufferAddLit(buf, "/>\n");
+}
+
static int
virDomainVideoDefFormat(virBufferPtr buf,
virDomainVideoDefPtr def,
@@ -26486,11 +26557,13 @@ virDomainVideoDefFormat(virBufferPtr buf,
virBufferAsprintf(buf, " heads='%u'", def->heads);
if (def->primary)
virBufferAddLit(buf, " primary='yes'");
-if (def->accel) {
+if (def->accel || def->res) {
virBufferAddLit(buf, ">\n");
virBufferAdjustIndent(buf, 2);
if (def->accel)
virDomainVideoAccelDefFormat(buf, def->accel);
+if (def->res)
+virDomainVideoResolutionDefFormat(buf, def->res);
virBufferAdjustIndent(buf, -2);
virBufferAddLit(buf, "\n");
} else {
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 33cef5b75c..a164f26d57 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1407,6 +1407,10 @@ struct _virDomainVideoAccelDef {
int accel3d; /* enum virTristateBool */
};
+struct _virDomainVideoResolutionDef {
+unsigned int x;
+unsigned int y;
+};
struct _virDomainVideoDriverDef {
virDomainVideoVGAConf vgaconf;
@@ -1422,6 +1426,7 @@ struct _virDomainVideoDef {
bool primary;
virDomainVideoAccelDefPtr accel;
virDomainVideoDriverDefPtr driver;
+virDomainVideoResolutionDefPtr res;
virDomainDeviceInfo info;
virDomainVirtioOptionsPtr virtio;
};
diff --git a/src/conf/virconftypes.h b/src/conf/virconftypes.h
index a15cfb5f9e..462842f324 100644
--- a/src/conf/virconftypes.h
+++ b/src/
[libvirt] [PATCH v2 02/10] qemu: Include {xres, yres} QEMU capabilities for video models
From: Julio Faracco
New QEMU capabilities for display resolution were added: xres and yres.
Some models supports like VGA, QXL, Virtio and Bochs support set
resolution for driver. The capabilities mentioned above were added into
those models.
Signed-off-by: Julio Faracco
---
src/qemu/qemu_capabilities.c | 16
src/qemu/qemu_capabilities.h | 2 ++
2 files changed, 18 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 73300128ea..e6d256a9cd 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -537,6 +537,8 @@ VIR_ENUM_IMPL(virQEMUCaps,
/* 335 */
"bochs-display",
"migration-file-drop-cache",
+ "xres",
+ "yres",
);
@@ -1219,6 +1221,8 @@ static struct virQEMUCapsStringFlags
virQEMUCapsDevicePropsKVMPit[] = {
static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsVGA[] = {
{ "vgamem_mb", QEMU_CAPS_VGA_VGAMEM },
+{ "xres", QEMU_CAPS_XRES },
+{ "yres", QEMU_CAPS_YRES },
};
static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsVmwareSvga[] = {
@@ -1229,6 +1233,8 @@ static struct virQEMUCapsStringFlags
virQEMUCapsDevicePropsQxl[] = {
{ "vgamem_mb", QEMU_CAPS_QXL_VGAMEM },
{ "vram64_size_mb", QEMU_CAPS_QXL_VRAM64 },
{ "max_outputs", QEMU_CAPS_QXL_MAX_OUTPUTS },
+{ "xres", QEMU_CAPS_XRES },
+{ "yres", QEMU_CAPS_YRES },
};
static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsVirtioGpu[] = {
@@ -1237,6 +1243,13 @@ static struct virQEMUCapsStringFlags
virQEMUCapsDevicePropsVirtioGpu[] = {
{ "disable-legacy", QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY },
{ "iommu_platform", QEMU_CAPS_VIRTIO_PCI_IOMMU_PLATFORM },
{ "ats", QEMU_CAPS_VIRTIO_PCI_ATS },
+{ "xres", QEMU_CAPS_XRES },
+{ "yres", QEMU_CAPS_YRES },
+};
+
+static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsBochs[] = {
+{ "xres", QEMU_CAPS_XRES },
+{ "yres", QEMU_CAPS_YRES },
};
static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsICH9[] = {
@@ -1362,6 +1375,9 @@ static virQEMUCapsObjectTypeProps
virQEMUCapsDeviceProps[] = {
{ "virtio-gpu-device", virQEMUCapsDevicePropsVirtioGpu,
ARRAY_CARDINALITY(virQEMUCapsDevicePropsVirtioGpu),
QEMU_CAPS_DEVICE_VIRTIO_GPU },
+{ "bochs-display", virQEMUCapsDevicePropsBochs,
+ ARRAY_CARDINALITY(virQEMUCapsDevicePropsBochs),
+ QEMU_CAPS_DEVICE_BOCHS_DISPLAY },
{ "ICH9-LPC", virQEMUCapsDevicePropsICH9,
ARRAY_CARDINALITY(virQEMUCapsDevicePropsICH9),
-1 },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 68ef6c49b4..5e3d542994 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -518,6 +518,8 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
syntax-check */
/* 335 */
QEMU_CAPS_DEVICE_BOCHS_DISPLAY, /* -device bochs-display */
QEMU_CAPS_MIGRATION_FILE_DROP_CACHE, /* migration with disk cache on is
safe for type='file' disks */
+QEMU_CAPS_XRES, /* -device *,xres= */
+QEMU_CAPS_YRES, /* -device *,yres= */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 09/10] tests: Introduce resolution test for Bochs model
From: Julio Faracco
New test case was added to cover 'xres' and 'yres' properties for Bochs
model due to latest QEMU version.
Signed-off-by: Julio Faracco
---
...ochs-display-resolution.x86_64-latest.args | 35 +++
.../video-bochs-display-resolution.xml| 33 +
tests/qemuxml2argvtest.c | 1 +
...bochs-display-resolution.x86_64-latest.xml | 33 +
tests/qemuxml2xmltest.c | 2 ++
5 files changed, 104 insertions(+)
create mode 100644
tests/qemuxml2argvdata/video-bochs-display-resolution.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/video-bochs-display-resolution.xml
create mode 100644
tests/qemuxml2xmloutdata/video-bochs-display-resolution.x86_64-latest.xml
diff --git
a/tests/qemuxml2argvdata/video-bochs-display-resolution.x86_64-latest.args
b/tests/qemuxml2argvdata/video-bochs-display-resolution.x86_64-latest.args
new file mode 100644
index 00..48e58d839f
--- /dev/null
+++ b/tests/qemuxml2argvdata/video-bochs-display-resolution.x86_64-latest.args
@@ -0,0 +1,35 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
+-machine pc,accel=tcg,usb=off,dump-guest-core=off \
+-m 1024 \
+-overcommit mem-lock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-boot strict=on \
+-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
+-device bochs-display,id=video0,vgamem=16384k,xres=1280,yres=720,\
+bus=pci.0,addr=0x2 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
+resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/video-bochs-display-resolution.xml
b/tests/qemuxml2argvdata/video-bochs-display-resolution.xml
new file mode 100644
index 00..f21d9955f3
--- /dev/null
+++ b/tests/qemuxml2argvdata/video-bochs-display-resolution.xml
@@ -0,0 +1,33 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 1048576
+ 1048576
+ 1
+
+hvm
+
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu-system-x86_64
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 2e46dbd177..f68c31d5e9 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2044,6 +2044,7 @@ mymain(void)
QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
QEMU_CAPS_VIRTIO_GPU_MAX_OUTPUTS);
DO_TEST_CAPS_LATEST("video-bochs-display-device");
+DO_TEST_CAPS_LATEST("video-bochs-display-resolution");
DO_TEST("video-none-device",
QEMU_CAPS_VNC);
DO_TEST_PARSE_ERROR("video-invalid-multiple-devices", NONE);
diff --git
a/tests/qemuxml2xmloutdata/video-bochs-display-resolution.x86_64-latest.xml
b/tests/qemuxml2xmloutdata/video-bochs-display-resolution.x86_64-latest.xml
new file mode 100644
index 00..f21d9955f3
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/video-bochs-display-resolution.x86_64-latest.xml
@@ -0,0 +1,33 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 1048576
+ 1048576
+ 1
+
+hvm
+
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu-system-x86_64
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 9e6c4345f1..3078355019 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1224,6 +1224,8 @@ mymain(void)
QEMU_CAPS_VNC,
QEMU_CAPS_DEVICE_VIRTIO_GPU_CCW);
DO_TEST_CAPS_LATEST("video-virtio-gpu-resolution");
+
+DO_TEST_CAPS_LATEST("video-bochs-display-resolution");
DO_TEST("video-none-device", NONE);
DO_TEST_CAPS_LATEST("intel-iommu");
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 07/10] tests: Introduce resolution test for QXL model
From: Julio Faracco
New test case was added to cover 'xres' and 'yres' properties for QXL
model due to latest QEMU version.
Signed-off-by: Julio Faracco
---
.../video-qxl-resolution.x86_64-latest.args | 35 +++
.../qemuxml2argvdata/video-qxl-resolution.xml | 33 +
tests/qemuxml2argvtest.c | 1 +
.../video-qxl-resolution.x86_64-latest.xml| 33 +
tests/qemuxml2xmltest.c | 1 +
5 files changed, 103 insertions(+)
create mode 100644
tests/qemuxml2argvdata/video-qxl-resolution.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/video-qxl-resolution.xml
create mode 100644
tests/qemuxml2xmloutdata/video-qxl-resolution.x86_64-latest.xml
diff --git a/tests/qemuxml2argvdata/video-qxl-resolution.x86_64-latest.args
b/tests/qemuxml2argvdata/video-qxl-resolution.x86_64-latest.args
new file mode 100644
index 00..73036521c7
--- /dev/null
+++ b/tests/qemuxml2argvdata/video-qxl-resolution.x86_64-latest.args
@@ -0,0 +1,35 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
+QEMU_AUDIO_DRV=none \
+/usr/bin/qemu-system-x86_64 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object secret,id=masterKey0,format=raw,\
+file=/tmp/lib/domain--1-QEMUGuest1/master-key.aes \
+-machine pc,accel=tcg,usb=off,dump-guest-core=off \
+-m 214 \
+-overcommit mem-lock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server,nowait \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-boot strict=on \
+-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
+-device qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,xres=1280,\
+yres=720,vram64_size_mb=0,vgamem_mb=8,max_outputs=1,bus=pci.0,addr=0x2 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \
+-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\
+resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/video-qxl-resolution.xml
b/tests/qemuxml2argvdata/video-qxl-resolution.xml
new file mode 100644
index 00..3ecdbedc02
--- /dev/null
+++ b/tests/qemuxml2argvdata/video-qxl-resolution.xml
@@ -0,0 +1,33 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219136
+ 219136
+ 1
+
+hvm
+
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu-system-x86_64
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 9395cc19a2..1c3e3a8487 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2014,6 +2014,7 @@ mymain(void)
QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
QEMU_CAPS_DEVICE_QXL,
QEMU_CAPS_QXL_MAX_OUTPUTS);
+DO_TEST_CAPS_LATEST("video-qxl-resolution");
DO_TEST("video-virtio-gpu-device",
QEMU_CAPS_DEVICE_VIRTIO_GPU,
QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
diff --git a/tests/qemuxml2xmloutdata/video-qxl-resolution.x86_64-latest.xml
b/tests/qemuxml2xmloutdata/video-qxl-resolution.x86_64-latest.xml
new file mode 100644
index 00..3ecdbedc02
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/video-qxl-resolution.x86_64-latest.xml
@@ -0,0 +1,33 @@
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 219136
+ 219136
+ 1
+
+hvm
+
+
+
+ destroy
+ restart
+ destroy
+
+/usr/bin/qemu-system-x86_64
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 5f6a3618cd..9ea94815ae 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1207,6 +1207,7 @@ mymain(void)
QEMU_CAPS_DEVICE_QXL);
DO_TEST("video-qxl-heads", NONE);
DO_TEST("video-qxl-noheads", NONE);
+DO_TEST_CAPS_LATEST("video-qxl-resolution");
DO_TEST("video-virtio-gpu-secondary", NONE);
DO_TEST("video-virtio-gpu-ccw",
QEMU_CAPS_CCW,
--
2.20.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 01/10] docs: Adding 'xres' and 'yres' into video XML definition
From: Julio Faracco This commit adds resolution element with parameters 'x' and 'y' into video XML domain group definition. Both, properties were added into an element called 'resolution' and it was added inside 'model' element. They are set as optional. This element does not follow QEMU properties 'xres' and 'yres' format. Signed-off-by: Julio Faracco --- docs/schemas/domaincommon.rng | 10 ++ 1 file changed, 10 insertions(+) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index c48f8c4f56..7b6468c36e 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3637,6 +3637,16 @@ + + + + + + + + + + -- 2.20.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 06/10] tests: Include bochs-display as capability test too
From: Julio Faracco
The display model 'bochs-display' should be added into capability test
case of command 'device-list-properties' because we are supporting this
display now. It is listed by 'virQEMUCapsDeviceProps'.
See src/qemu/qemu_capabilities.c:1378.
Signed-off-by: Julio Faracco
---
.../caps_3.0.0.ppc64.replies | 139 +++
.../caps_3.0.0.x86_64.replies | 167 +-
.../caps_3.1.0.ppc64.replies | 139 +++
.../caps_3.1.0.x86_64.replies | 167 +-
.../caps_4.0.0.aarch64.replies| 139 +++
.../caps_4.0.0.ppc64.replies | 139 +++
.../caps_4.0.0.riscv32.replies| 131 +++---
.../caps_4.0.0.riscv64.replies| 131 +++---
.../caps_4.0.0.x86_64.replies | 167 +-
.../caps_4.1.0.x86_64.replies | 159 -
10 files changed, 1114 insertions(+), 364 deletions(-)
diff --git a/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.replies
b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.replies
index fa370a39bd..c3c9cdac53 100644
--- a/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.replies
+++ b/tests/qemucapabilitiesdata/caps_3.0.0.ppc64.replies
@@ -4963,13 +4963,88 @@
{
"execute": "device-list-properties",
"arguments": {
-"typename": "ICH9-LPC"
+"typename": "bochs-display"
},
"id": "libvirt-24"
}
{
- "id": "libvirt-24",
+ "return": [
+{
+ "name": "yres",
+ "type": "uint32"
+},
+{
+ "name": "xmax",
+ "type": "uint32"
+},
+{
+ "name": "big-endian-framebuffer",
+ "type": "bool"
+},
+{
+ "name": "addr",
+ "description": "Slot and optional function number, example: 06.0 or 06",
+ "type": "int32"
+},
+{
+ "name": "vgamem",
+ "type": "size"
+},
+{
+ "name": "edid",
+ "type": "bool"
+},
+{
+ "name": "ymax",
+ "type": "uint32"
+},
+{
+ "name": "x-pcie-extcap-init",
+ "description": "on\/off",
+ "type": "bool"
+},
+{
+ "name": "x-pcie-lnksta-dllla",
+ "description": "on\/off",
+ "type": "bool"
+},
+{
+ "name": "command_serr_enable",
+ "description": "on\/off",
+ "type": "bool"
+},
+{
+ "name": "multifunction",
+ "description": "on\/off",
+ "type": "bool"
+},
+{
+ "name": "romfile",
+ "type": "str"
+},
+{
+ "name": "rombar",
+ "type": "uint32"
+},
+{
+ "name": "xres",
+ "type": "uint32"
+}
+ ],
+ "id": "libvirt-24"
+}
+
+{
+ "execute": "device-list-properties",
+ "arguments": {
+"typename": "ICH9-LPC"
+ },
+ "id": "libvirt-25"
+}
+
+{
+ "id": "libvirt-25",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'ICH9-LPC' not found"
@@ -4981,7 +5056,7 @@
"arguments": {
"typename": "virtio-balloon-pci"
},
- "id": "libvirt-25"
+ "id": "libvirt-26"
}
{
@@ -5124,7 +5199,7 @@
"type": "str"
}
],
- "id": "libvirt-25"
+ "id": "libvirt-26"
}
{
@@ -5132,11 +5207,11 @@
"arguments": {
"typename": "virtio-balloon-ccw"
},
- "id": "libvirt-26"
+ "id": "libvirt-27"
}
{
- "id": "libvirt-26",
+ "id": "libvirt-27",
"error": {
"class": "DeviceNotFound",
"desc": "Device 'virtio-balloon-ccw' not found"
@@ -5148,7 +5223,7 @@
"arguments": {
"typename": "virtio-balloon-device"
},
- "id": "libvirt-27"
+ "id": "libvirt-28"
}
{
@@ -5192,7 +5267,7 @@
"type": "bool"
}
],
- "id": "libvirt-27"
+ "id": "libvirt-28"
}
{
@@ -5200,7 +5275,7 @@
"arguments": {
"typename": "nec-usb-xhci"
},
- "id": "libvirt-28"
+ "id": "libvirt-29"
}
{
@@ -5280,7 +5355,7 @@
"type": "uint32"
}
],
- "id": "libvirt-28"
+ "id": "libvirt-29"
}
{
@@ -5288,7 +5363,7 @@
"arguments": {
"typename": "spapr-pci-host-bridge"
},
- "id": "libvirt-29"
+ "id": "libvirt-30"
}
{
@@ -5346,7 +5421,7 @@
"type": "uint32"
}
],
- "id": "libvirt-29"
+ "id": "libvirt-30"
}
{
@@ -5354,7 +5429,7 @@
"arguments": {
"typename": "memory-backend-file"
},
- "id": "libvirt-30"
+ "id": "libvirt-31"
}
{
@@ -5404,7 +5479,7 @@
"type": "string"
}
],
- "id": "libvirt-30"
+ "id": "libvirt-31"
}
{
@@ -5412,7 +5487,7 @@
"arguments": {
"typename": "memory-backend-memfd"
},
- "id": "libvirt-31"
+ "id": "libvirt-32"
}
{
@@ -5462,12 +5537,12 @@
"type": "string"
}
],
- "id": "libvirt-31"
+ "id": "libvirt-32"
}
{
"execute": "query-machines",
- "id": "libvirt-32"
+ "id": "libvirt-33"
}
{
@@ -5610,7 +5685,7 @@
"cpu-max": 1
}
],
- "id": "libvirt-32"
+ "id": "libvirt-33"
}
{
@@ -5618,7 +5693,7 @@
"arguments": {
"typename": "pseries-3.0-machine"
},
- "id"
Re: [libvirt] Fwd: libvirtd failing on MacOS in setgroups
Hi Eric, > I wonder if gnulib could provide a workaround setgroups() that overcomes > this issue I don't see how a workaround could look like. The problem is not the value of NGROUPS_MAX in user-space, but the same value NGROUPS_MAX in the kernel. More precisely, in the Darwin kernel file bsd/kern/kern_prot.c there is a function 'setgroups1', that contains the common implementation of the setgroups() and initgroups() system call, and this function fails with EINVAL if the number of groups in the set is > NGROUPS. In the kernel sources, NGROUPS is defined as NGROUPS_MAX, and NGROUPS_MAX is defined as 16. So, the situation on macOS has not changed since this page was written: https://www.j3e.de/ngroups.html What kind of workaround are you imagining? That we override open(), access(), eaccess() to call setgroups() first, in an intelligent way? That would be quite gross. For what purpose is libvirt or QEMU using setgroups()? Bruno -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC] On present using dummy hostdev usb device
On 8/30/19 5:19 PM, Daniel P. Berrangé wrote: > On Fri, Aug 30, 2019 at 05:08:52PM +0200, Michal Privoznik wrote: >> On 8/30/19 2:30 PM, Roman Kagan wrote: >>> On Fri, Aug 30, 2019 at 10:09:06AM +0100, Daniel P. Berrangé wrote: On Fri, Aug 30, 2019 at 08:44:03AM +, Nikolay Shirokovskiy wrote: > Hi, all! > >We use an interesting approach when starting/migrating/etc domain with > usb > hostdev with startupPolicy=optional. We add qemu usb-host device with > missing hostaddr/hostbus parameters (dummy device). I guess there are > 2 reasons why we do it. First without dummy device migration will fail as > described in [1]. Second is an interesting property of dummy device that > qemu starts to monitor for attaching of usb devices and binds the first > attached to node to the dummy device. So one can start a domain with > missing hostdev and attach it later or migrate a domain then detach > hostdev on source and attach it on destination. But as qemu binds the > first attached device this is not reliable, to say the least. And after > all this does not work if domain uses distinct mount namespace which > is default. Even without mount namespaces, it should fail as QEMU is running non-root and libvirt won't have granted access to any host USB devices in /dev, and also SELinux policy will forbid this. >>> >>> Right, but the case with mount namespaces is particularly problematic: >>> if the device open fails due to missing device node, libusb removes the >>> device from its internal device list. This results in the following >>> scenario: >>> >>> - libvirt adds a dummy usb-host device to QEMU in place of a missing >>>device >>> >>> - QEMU (via libusb) installs a watch for udev add events >>> >>> - the physical device is plugged into the host >>> >>> - QEMU detects the addition of the device and, since the dummy device >>>matches everything, tries to open it >>> >>> - by this time libvirt may have not created a device node in QEMU's >>>mount namespace, so the open fails due to missing device node, and >>>libusb removes the device from its internal list >>> >>> - libvirt removes the dummy usb-host device and adds the actual usb-host >>>device >>> >>> - QEMU fails to open it because it's no longer seen by libusb >> >> There is a bug filed against libusb exactly for this: >> >> https://bugzilla.redhat.com/show_bug.cgi?id=1595525 >> >> BTW: you don't have to migrate, it's sufficient to start a domain with a >> missing USB and startupPolicy='optional' and then physically plug it into >> the host and then try to hotplug it into the domain. > > AFAIR, the startupPolicy=optional was never really intended to allow > for the device to be dynamically added after startup. It was only > trying drop the device if it didn't exist at startup. Sure, I'm not saying that. That's why I'm saying explicitly you need to hotplug the device that was misssing. I'm only supporting our case of not adding dummy device to libvirt since there's a bug filed against libusb that exhibits same symptoms. Perhaps I should have said that more clearly. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
