[PATCH] qemu: forbid updating any attributes of an interface <backend> with update-device

[Laine Stump]

On 2/21/23 3:11 AM, Martin Kletzander wrote:
> On Wed, Feb 15, 2023 at 04:06:41PM -0500, Laine Stump wrote:
>> Changing any of the attributes of an <interface>'s <backend> would
>> require removing and re-adding the interface for the new setting to
>> take effect, so fail any update-device that changes anything in
>> <backend>
>>
>> Resolves: https://bugzilla.redhat.com/2169245
>> Signed-off-by: Laine Stump <laine at redhat.com>
>> ---
>> src/conf/domain_conf.c   | 14 ++++++++++++++
>> src/conf/domain_conf.h   |  2 ++
>> src/libvirt_private.syms |  1 +
>> src/qemu/qemu_hotplug.c  |  9 +++++++++
>> 4 files changed, 26 insertions(+)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 3dfc5c87af..aa7bed7dc3 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -19853,6 +19853,20 @@ 
>> virDomainFsDefCheckABIStability(virDomainFSDef *src,
>> }
>>
>>
>> +bool
>> +virDomainNetBackendIsEqual(virDomainNetBackend *src,
>> +                           virDomainNetBackend *dst)
>> +{
>> +    if (src->type != dst->type ||
>> +        STRNEQ_NULLABLE(src->tap, dst->tap) ||
>> +        STRNEQ_NULLABLE(src->vhost, dst->vhost) ||
>> +        STRNEQ_NULLABLE(src->logFile, dst->logFile)) {
> 
> I thought you are missing the @upstream member comparison here, but it
> looks like it is not used anywhere in the code.

Oh right! I noticed the same thing when I noticed this, and had a 1st 
patch that removed the upstream member, along with pointing out how it 
got there and why it is no longer needed, but I had forgotten about it 
by the time I sent the other patch, and continued to forget about it 
when I sent the ping :-/

But I see you already figured that out, and sent a patch to remove it, 
while I was sleeping. Thanks!

   So I guess
> 
> Reviewed-by: Martin Kletzander <mkletzan at redhat.com>

and thanks!

> 
> and I'll do some grave-digging about the long lost member.

And sorry for making you spend the time to go digging.

> 
> [...]
> 
>> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
>> index c490e2b97a..b4cddef9f5 100644
>> --- a/src/qemu/qemu_hotplug.c
>> +++ b/src/qemu/qemu_hotplug.c
>> @@ -3675,6 +3675,15 @@ qemuDomainChangeNet(virQEMUDriver *driver,
>>         goto cleanup;
>>     }
>>
>> +    /* nothing in <backend> can be modified in an existing interface -
>> +     * the entire device will need to be removed and re-added.
> 
> If I were a nit picker I could tell you to start the sentence with an
> uppercase letter or remove the full stop.  O:-)
> 
>> +     */
>> +    if (!virDomainNetBackendIsEqual(&olddev->backend, 
>> &newdev->backend)) {
>> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
>> +                       _("cannot modify network device backend 
>> settings"));
>> +        goto cleanup;
>> +    }
>> +
>>     /* allocate new actual device to compare to old - we will need to
>>      * free it if we fail for any reason
>>      */
>> -- 
>> 2.39.1
>>