[PATCH v2 1/2] qemu: check for/require shared memory for *any* vhostuser interface

Laine Stump posted 2 patches 6 months, 2 weeks ago
[PATCH v2 1/2] qemu: check for/require shared memory for *any* vhostuser interface
Posted by Laine Stump 6 months, 2 weeks ago
It has always been true that vhostuser interfaces require shared
memory, but we've never had that check in the validation. Recently I
added that check for the case of interface type='vhostuser' backend
type='passt'. This patch generalizes that check to require shared
memory for *any* vhostuser interface.

(While it is true that we've been allowing interface type='vhostuser'
without shared memory enabled for 11 years or so, so there might be
some existing config (or config-generating script) that does that and
would be broken by this new validation, it is also true that such a
config could never have worked, so it was already broken (just failing
at runtime rather than during parsing of the config)).

Signed-off-by: Laine Stump <laine@redhat.com>
---
new patch for V2

 src/qemu/qemu_validate.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index f3ef1be660..2479596628 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -1820,9 +1820,8 @@ qemuValidateDomainDeviceDefNetwork(const virDomainNetDef *net,
         return -1;
     }
 
-    if (net->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER &&
-        net->backend.type == VIR_DOMAIN_NET_BACKEND_PASST) {
-        if (qemuValidateDomainDefVhostUserRequireSharedMemory(def, "interface type=\"vhostuser\" backend type=\"passt\"") < 0)
+    if (net->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
+        if (qemuValidateDomainDefVhostUserRequireSharedMemory(def, "vhostuser") < 0)
             return -1;
     }
 
-- 
2.48.1
Re: [PATCH v2 1/2] qemu: check for/require shared memory for *any* vhostuser interface
Posted by Michal Prívozník 6 months, 2 weeks ago
On 2/21/25 20:05, Laine Stump wrote:
> It has always been true that vhostuser interfaces require shared
> memory, but we've never had that check in the validation. Recently I
> added that check for the case of interface type='vhostuser' backend
> type='passt'. This patch generalizes that check to require shared
> memory for *any* vhostuser interface.
> 
> (While it is true that we've been allowing interface type='vhostuser'
> without shared memory enabled for 11 years or so, so there might be
> some existing config (or config-generating script) that does that and
> would be broken by this new validation, it is also true that such a
> config could never have worked, so it was already broken (just failing
> at runtime rather than during parsing of the config)).
> 
> Signed-off-by: Laine Stump <laine@redhat.com>
> ---
> new patch for V2
> 
>  src/qemu/qemu_validate.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index f3ef1be660..2479596628 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -1820,9 +1820,8 @@ qemuValidateDomainDeviceDefNetwork(const virDomainNetDef *net,
>          return -1;
>      }
>  
> -    if (net->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER &&
> -        net->backend.type == VIR_DOMAIN_NET_BACKEND_PASST) {
> -        if (qemuValidateDomainDefVhostUserRequireSharedMemory(def, "interface type=\"vhostuser\" backend type=\"passt\"") < 0)
> +    if (net->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
> +        if (qemuValidateDomainDefVhostUserRequireSharedMemory(def, "vhostuser") < 0)
>              return -1;
>      }
>  

This relaxed check breaks some tests:

Summary of Failures:

162/295 libvirt:bin / qemusecuritytest
     FAIL            1.78s   exit status 1
261/295 libvirt:bin / qemuxmlconftest
     FAIL           10.24s   exit status 1


Michal
Re: [PATCH v2 1/2] qemu: check for/require shared memory for *any* vhostuser interface
Posted by Laine Stump 6 months, 2 weeks ago
On 2/24/25 7:27 AM, Michal Prívozník wrote:
> On 2/21/25 20:05, Laine Stump wrote:
>> It has always been true that vhostuser interfaces require shared
>> memory, but we've never had that check in the validation. Recently I
>> added that check for the case of interface type='vhostuser' backend
>> type='passt'. This patch generalizes that check to require shared
>> memory for *any* vhostuser interface.
>>
>> (While it is true that we've been allowing interface type='vhostuser'
>> without shared memory enabled for 11 years or so, so there might be
>> some existing config (or config-generating script) that does that and
>> would be broken by this new validation, it is also true that such a
>> config could never have worked, so it was already broken (just failing
>> at runtime rather than during parsing of the config)).
>>
>> Signed-off-by: Laine Stump <laine@redhat.com>
>> ---
>> new patch for V2
>>
>>   src/qemu/qemu_validate.c | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
>> index f3ef1be660..2479596628 100644
>> --- a/src/qemu/qemu_validate.c
>> +++ b/src/qemu/qemu_validate.c
>> @@ -1820,9 +1820,8 @@ qemuValidateDomainDeviceDefNetwork(const virDomainNetDef *net,
>>           return -1;
>>       }
>>   
>> -    if (net->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER &&
>> -        net->backend.type == VIR_DOMAIN_NET_BACKEND_PASST) {
>> -        if (qemuValidateDomainDefVhostUserRequireSharedMemory(def, "interface type=\"vhostuser\" backend type=\"passt\"") < 0)
>> +    if (net->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
>> +        if (qemuValidateDomainDefVhostUserRequireSharedMemory(def, "vhostuser") < 0)
>>               return -1;
>>       }
>>   
> 
> This relaxed check breaks some tests:
> 
> Summary of Failures:
> 
> 162/295 libvirt:bin / qemusecuritytest
>       FAIL            1.78s   exit status 1
> 261/295 libvirt:bin / qemuxmlconftest
>       FAIL           10.24s   exit status 1

Oops, I remember thinking "this is going to break some test cases", and 
then later somehow tricked myself into believing that I'd run the tests 
and fixed them. I''l correct that now and re-send.