- Devel - libvirt List Archives

[PATCH 0/2] qemuxml2argvtest: Don't exec '/usr/libexec/qemu/vhost-user/test-vhost-user-gpu'
by Peter Krempa 20 Dec '23

20 Dec '23

'qemuExtVhostUserGPUPrepareDomain' breaks our design assumptions about the 'PrepareDomain' step which should not touch anything on the host. This patchset for now fixes the symptom by mocking the function and poisons virFork and virCommandRun so that this doesn't happen in the future. Proper fix will require splitting the vhost-user GPU prepare step to prepare the host-specific portion separately. Peter Krempa (2): qemuxml2argvmock: Mock qemuExtVhostUserGPUPrepareDomain qemuxml2argvmock: Poison virCommandRun and virFork from test context src/qemu/qemu_vhost_user_gpu.c | 4 ++++ src/qemu/qemu_vhost_user_gpu.h | 2 +- src/util/vircommand.h | 4 ++-- tests/qemuxml2argvmock.c | 28 ++++++++++++++++++++++++++++ 4 files changed, 35 insertions(+), 3 deletions(-) -- 2.43.0

2 7

[PATCH] conf: Remove multiplication to avoid overflow
by Egor Makrushin 20 Dec '23

20 Dec '23

Multiplication results in integer overflow. Replace value of 6th agrument with ULLONG_MAX. Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: 04bd77a19f ("conf: Move and rename virDomainParseScaledValue()") Signed-off-by: Egor Makrushin <emakrushin(a)astralinux.ru> --- src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 58a985fc5d..871fd3a874 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -8523,7 +8523,7 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt, unsigned long long bytes; if ((rc = virParseScaledValue("./pcihole64", NULL, ctxt, &bytes, 1024, - 1024ULL * ULONG_MAX, false)) < 0) + ULLONG_MAX, false)) < 0) return NULL; if (rc == 1) -- 2.30.2

3 3

[libvirt PATCH] remote: DeserializeDomainDiskErrors: remove dead code
by Ján Tomko 19 Dec '23

19 Dec '23

As of commit b2d079c113a which converted this function to use g_strdup, the error label is only reached when i = 0, rendering it useless. Remove it. Fixes: https://gitlab.com/libvirt/libvirt/-/issues/572 Signed-off-by: Ján Tomko <jtomko(a)redhat.com> --- src/remote/remote_driver.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 132d0194c6..392377deae 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1641,12 +1641,11 @@ remoteDeserializeDomainDiskErrors(remote_domain_disk_error *ret_errors_val, int maxerrors) { size_t i = 0; - size_t j; if (ret_errors_len > limit || ret_errors_len > maxerrors) { virReportError(VIR_ERR_RPC, "%s", _("returned number of disk errors exceeds limit")); - goto error; + return -1; } for (i = 0; i < ret_errors_len; i++) { @@ -1655,12 +1654,6 @@ remoteDeserializeDomainDiskErrors(remote_domain_disk_error *ret_errors_val, } return 0; - - error: - for (j = 0; j < i; j++) - VIR_FREE(errors[j].disk); - - return -1; } static int -- 2.42.0

2 1

[PATCH] apparmor: Add capabilities for PCI passthrough to virtxend profile
by Jim Fehlig 19 Dec '23

19 Dec '23

When splitting out the apparmor modular daemon profiles from the libvirtd profile, the net_admin and sys_admin capabilities were dropped from the virtxend profile. It was not known at the time that these capabilities were needed for PCI passthrough. Without the capabilities, the following messages are emitted from the audit subsystem audit: type=1400 audit(1702939277.946:63): apparmor="DENIED" \ operation="capable" class="cap" profile="virtxend" pid=3611 \ comm="rpc-virtxend" capability=21 capname="sys_admin" audit: type=1400 audit(1702940304.818:63): apparmor="DENIED" \ operation="capable" class="cap" profile="virtxend" pid=3731 \ comm="rpc-virtxend" capability=12 capname="net_admin" It appears sys_admin is needed to simply read from the PCI dev's sysfs config file. The net_admin capability is needed when setting the MAC address of an SR-IOV virtual function. Signed-off-by: Jim Fehlig <jfehlig(a)suse.com> --- src/security/apparmor/usr.sbin.virtxend.in | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/security/apparmor/usr.sbin.virtxend.in b/src/security/apparmor/usr.sbin.virtxend.in index 78a11305f5..77fedce352 100644 --- a/src/security/apparmor/usr.sbin.virtxend.in +++ b/src/security/apparmor/usr.sbin.virtxend.in @@ -5,8 +5,10 @@ profile virtxend @sbindir@/virtxend flags=(attach_disconnected) { #include <abstractions/dbus> capability kill, + capability net_admin, capability setgid, capability setuid, + capability sys_admin, capability sys_pacct, capability ipc_lock, -- 2.43.0

2 1

Patches on libvirt patchew are outdated
by Han Han 19 Dec '23

19 Dec '23

Hi, The libvirt patchew https://patchew.org/Libvirt/ has no update for more than one month. Is it dropped? If not, please sync the patches to it~

3 6

[vf-token 0/8] Introduce vf-token when using userspace PF
by Vivek Kashyap 18 Dec '23

18 Dec '23

The VFIO PCI ABI has been extended to require userspace PF driver to set a VF token to a known value. The VF drivers are then required to provide this token to access the VF device. The vf-token is set by the PF driver before VF drivers can access the device. The kernel provides no means to retrieve the token in use; but there is no specification describing the distribution or level of confidentiality of the token. Qemu has been extended to require the vf-token when vf device is used. An important point to note is that the vf-token is required only when both the PF and VF are used in userspace. This patch series adds support to provide the vf-token (uuid format) in the domain XML and to generate the qemu commandline including the vf-token. To support vf-token the new element will be used as follows: <hostdev mode='subsystem' type='pci' managed='yes'> <driver name='vfio'/> <source> <address domain='0x0000' bus='0x0' slot='0x00' function='0x1'> <vf-token uuid='00112233-4455-6677-8899-aabbccddeeff'/> </address> </source> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> </hostdev> The generated commandline will include the following: -device {"driver":"vfio-pci","host":"0000:00:0.1", "vf-token":"00112233-4455-6677-8899-aabbccddeeff", "id":"hostdev0","bus":"pci.0","addr":"0x1"} Changes since initial RFC: 1. Added documentation 2. Added test cases and ran successful test suite after each patch commit 3. fixed spaces, coding sytle, and uuid string format 4. Used S:vftoken in virJSONValueObjectAdd instead of a conditional Vivek Kashyap (8): Define the vf-token extension for PCI device Introduce the vf-token qemu capability This patch introduces the PCI address extension flag for vf-token This patch introduces new XML parser/formatter functions for parsing the vf-token Introduce a validation function for vf-token support in qemu and generate vf-token device attribute in qemu command line Provide information about the vf-token flag Add tests for the vf-token flag to the qemuxml2argv and qemuxml2xml test suites Update news about vf-token NEWS.rst | 8 +++ docs/formatdomain.rst | 3 ++ src/conf/device_conf.c | 49 ++++++++++++++++--- src/conf/domain_addr.h | 1 + src/conf/domain_conf.c | 8 +++ src/conf/schemas/basictypes.rng | 7 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 3 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 8 +++ src/qemu/qemu_domain_address.c | 3 ++ src/qemu/qemu_validate.c | 20 ++++++++ src/util/virpci.c | 7 +++ src/util/virpci.h | 10 ++++ .../qemucapabilitiesdata/caps_8.1.0_s390x.xml | 1 + .../caps_8.1.0_x86_64.xml | 1 + .../caps_8.2.0_x86_64.xml | 1 + .../hostdev-vfio-vf-token.x86_64-latest.args | 34 +++++++++++++ .../hostdev-vfio-vf-token.xml | 22 +++++++++ tests/qemuxml2argvtest.c | 1 + .../hostdev-vfio-vf-token.x86_64-latest.xml | 40 +++++++++++++++ tests/qemuxml2xmltest.c | 1 + 22 files changed, 223 insertions(+), 7 deletions(-) create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-vf-token.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-vf-token.xml create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-vf-token.x86_64-latest.xml -- 2.33.8

1 8

[PATCH] rpc: fix race in waking up client event loop
by Daniel P. Berrangé 18 Dec '23

18 Dec '23

The first thread to issue a client RPC request will own the event loop execution, sitting in the virNetClientIOEventLoop function. It releases the client lock while running: virNetClientUnlock() g_main_loop_run() virNetClientLock() If a second thread arrives with an RPC request, it will queue it for the first thread to process. To inform the first thread that there's a new request it calls g_main_loop_quit() to break it out of the main loop. This works if the first thread is in g_main_loop_run() at that time. There is a small window of opportunity, however, where the first thread has released the client lock, but not yet got into g_main_loop_run(). If that happens, the wakeup from the second thread is lost. This patch deals with that by changing the way the wakeup is performed. Instead of directly calling g_main_loop_quit(), the second thread creates an idle source to run the quit function from within the first thread. This guarantees that the first thread will see the wakeup. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com> --- src/rpc/virnetclient.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 4ab8af68c5..68098b1c8d 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -1848,6 +1848,15 @@ static void virNetClientIOUpdateCallback(virNetClient *client, } +static gboolean virNetClientIOWakeup(gpointer opaque) +{ + GMainLoop *loop = opaque; + + g_main_loop_quit(loop); + + return G_SOURCE_REMOVE; +} + /* * This function sends a message to remote server and awaits a reply * @@ -1925,7 +1934,9 @@ static int virNetClientIO(virNetClient *client, /* Check to see if another thread is dispatching */ if (client->haveTheBuck) { /* Force other thread to wakeup from poll */ - g_main_loop_quit(client->eventLoop); + GSource *wakeup = g_idle_source_new(); + g_source_set_callback(wakeup, virNetClientIOWakeup, client->eventLoop, NULL); + g_source_attach(wakeup, client->eventCtx); /* If we are non-blocking, detach the thread and keep the call in the * queue. */ -- 2.43.0

4 3

[PATCH 1/3] rpc: mark GSourceFuncs functions in vireventglibwatch.c as static
by Fima Shevrin 18 Dec '23

18 Dec '23

From: "Denis V. Lunev" <den(a)openvz.org> They are not exported from the module and thus should be static. Signed-off-by: Denis V. Lunev <den(a)openvz.org> Signed-off-by: Fima Shevrin <efim.shevrin(a)virtuozzo.com> --- src/util/vireventglibwatch.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/vireventglibwatch.c b/src/util/vireventglibwatch.c index b7f3a8786a..b21e505731 100644 --- a/src/util/vireventglibwatch.c +++ b/src/util/vireventglibwatch.c @@ -71,7 +71,7 @@ virEventGLibFDSourceFinalize(GSource *source G_GNUC_UNUSED) } -GSourceFuncs virEventGLibFDSourceFuncs = { +static GSourceFuncs virEventGLibFDSourceFuncs = { .prepare = virEventGLibFDSourcePrepare, .check = virEventGLibFDSourceCheck, .dispatch = virEventGLibFDSourceDispatch, @@ -194,7 +194,7 @@ virEventGLibSocketSourceFinalize(GSource *source) } -GSourceFuncs virEventGLibSocketSourceFuncs = { +static GSourceFuncs virEventGLibSocketSourceFuncs = { .prepare = virEventGLibSocketSourcePrepare, .check = virEventGLibSocketSourceCheck, .dispatch = virEventGLibSocketSourceDispatch, -- 2.34.1

5 12

[PATCH (pushed)] qemuDomainBlockPeek: Fix format checking logic
by Peter Krempa 16 Dec '23

16 Dec '23

Recent refactor which changed the format check to use qemuBlockStorageSourceIsRaw accidentaly inverted the condition. Caught by the CI test suite. Fixes: b600b69f8291226095e0fdb2eb7503baa79063fc Signed-off-by: Peter Krempa <pkrempa(a)redhat.com> --- Pushed as trivial CI fix. src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9d8be15721..2d75357939 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10173,7 +10173,7 @@ qemuDomainBlockPeek(virDomainPtr dom, goto cleanup; } - if (qemuBlockStorageSourceIsRaw(disk->src)) { + if (!qemuBlockStorageSourceIsRaw(disk->src)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("peeking is only supported for disk with 'raw' format not '%1$s'"), virStorageFileFormatTypeToString(disk->src->format)); -- 2.43.0

1 0

Re: [PATCH 3/3] rpc: Rework rpc notifications in main and side thread
by Daniel P. Berrangé 15 Dec '23

15 Dec '23

On Fri, Dec 15, 2023 at 01:48:09PM +0000, Efim Shevrin wrote: > Hello, > > here are call traces with two threads generated by python script > // ============================== > [root@dandreev-vz-6-0-0-243-master libvirt]# gdb -p 288985 > (gdb) t a a bt > > Thread 2 (Thread 0x7f2112862640 (LWP 288986) "python3"): > #0 0x00007f2121d4296f in poll () at /lib64/libc.so.6 > #1 0x00007f211444650c in g_main_context_iterate.constprop () at /lib64/libglib-2.0.so.0 > #2 0x00007f21143f1483 in g_main_loop_run () at /lib64/libglib-2.0.so.0 > #3 0x00007f211406800b in virNetClientIOEventLoop () at /lib64/libvirt.so.0 > #4 0x00007f2114068a0a in virNetClientIO () at /lib64/libvirt.so.0 > #5 0x00007f21140692c1 in virNetClientSendInternal () at /lib64/libvirt.so.0 > #6 0x00007f211406936d in virNetClientSendWithReply () at /lib64/libvirt.so.0 > #7 0x00007f2114061a1d in virNetClientProgramCall () at /lib64/libvirt.so.0 > #8 0x00007f21140f79ac in callFull () at /lib64/libvirt.so.0 > #9 0x00007f21140f7a2f in call () at /lib64/libvirt.so.0 > #10 0x00007f21140d8435 in remoteDomainCreate () at /lib64/libvirt.so.0 > #11 0x00007f21141dd60c in virDomainCreate () at /lib64/libvirt.so.0 > #12 0x00007f21145c8114 in libvirt_virDomainCreate () at /usr/lib64/python3.9/site-packages/libvirtmod.cpython-39-x86_64-linux-gnu.so Ok, this thread is the primary one responsible for I/O. It is waiting, while not holding any mutex. > Thread 1 (Thread 0x7f21223cf740 (LWP 288985) "python3"): > #0 0x00007f2121c9c39a in __futex_abstimed_wait_common () at /lib64/libc.so.6 > #1 0x00007f2121c9eba0 in pthread_cond_wait@@GLIBC_2.3.2 () at /lib64/libc.so.6 > #2 0x00007f2113f4982a in virCondWait () at /lib64/libvirt.so.0 > #3 0x00007f2113f1fee3 in virObjectWait () at /lib64/libvirt.so.0 > #4 0x00007f211406882a in virNetClientIO () at /lib64/libvirt.so.0 > #5 0x00007f21140692c1 in virNetClientSendInternal () at /lib64/libvirt.so.0 > #6 0x00007f211406936d in virNetClientSendWithReply () at /lib64/libvirt.so.0 > #7 0x00007f2114061a1d in virNetClientProgramCall () at /lib64/libvirt.so.0 > #8 0x00007f21140f79ac in callFull () at /lib64/libvirt.so.0 > #9 0x00007f21140f7a2f in call () at /lib64/libvirt.so.0 > #10 0x00007f21140f24eb in remoteNodeDeviceNumOfCaps () at /lib64/libvirt.so.0 > #11 0x00007f2114207a00 in virNodeDeviceNumOfCaps () at /lib64/libvirt.so.0 This is a second API call that has arrived and has queued its outgoing message, and is waiting either for the other thread to pass ownership of the socket to it, or for the other thread to provide its reply (whichever comes first). This is all totally normal, and working as expected, so I still don't see what the actual problem is ? > // ========================================================================= > > just in case here is python script > > [root@dandreev-vz-6-0-0-243-master ~]# cat a.py > import libvirt > import time > from threading import Thread > > def startVM(connection, vm_name): > try: > # Find the virtual machine by name > print('starting VM') > connection.lookupByName(vm_name).create() > print('done starting VM') > except libvirt.libvirtError as e: > print(f'Libvirt Error: {e}') > > # Replace 'qemu+tcp://10.34.66.13/system' with your actual connection URI > connection_uri = 'qemu+tcp://localhost/system' > connection = libvirt.open(connection_uri) > if connection is None: > print(f'Failed to open connection to {connection_uri}') > exit(1) > > try: > # Replace 'your_vm_name' with the actual name of your virtual machine > # startVM(connection, 'instance-00000002') > thread = Thread(target=lambda: startVM(connection, 'instance-00000002')) > thread.start() > time.sleep(3) > devs = connection.listAllDevices() > for i in range(100000): > for dev in devs: > print('listing caps for dev %i %s' % (i, dev)) > try: > dev.listCaps() > except Exception as e: > print('ERROR: %s' % e) > print('DONE listing caps') > > connection.lookupByName('instance-00000002').destroy() > except: > connection.lookupByName('instance-00000002').destroy() > finally: > thread.join() > # Close the connection outside the function > connection.close() What behaviour are you expecting this demo program to show ? When I run it, the VM starts and the the dev caps listing is then run 3 seconds later (due to the sleep). Removing the sleep, I can see the caps list running in parallel with the 'create'. Basically I see nothing wrong. With 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 :|

2 1