Re: [PATCH] qemu: forbid updating any attributes of an interface <backend> with update-device
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(a)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(a)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
>