On Thu, Mar 6, 2025 at 11:57 PM Peter Krempa <pkre...@redhat.com> wrote:
> On Sat, Feb 15, 2025 at 16:35:52 +0800, yong.hu...@smartx.com wrote:
> > From: Hyman Huang <yong.hu...@smartx.com>
> >
> > Support hotplug/hotunplug of virtual CPU by wrapping the
> > existing interface qemuDomainSetVcpu.
> >
> > Signed-off-by: Hyman Huang <yong.hu...@smartx.com>
> > ---
> > include/libvirt/libvirt-domain.h | 12 ++++++++++++
> > src/qemu/qemu_driver.c | 29 +++++++++++++++++++++++++++++
> > 2 files changed, 41 insertions(+)
> >
> > diff --git a/include/libvirt/libvirt-domain.h
> b/include/libvirt/libvirt-domain.h
> > index ae1e07b7b6..16e3140e09 100644
> > --- a/include/libvirt/libvirt-domain.h
> > +++ b/include/libvirt/libvirt-domain.h
> > @@ -6612,6 +6612,18 @@ virDomainGraphicsReload(virDomainPtr domain,
> > unsigned int type,
> > unsigned int flags);
> >
> > +/* Virtual CPU set parameters */
> > +
> > +/**
> > + * VIR_DOMAIN_VCPU_STATE:
> > + *
> > + * The tunable enable/disable individual vcpus described by @vcpumap
> > + * in the hypervisor*
> > + *
> > + * Since: 11.1.0
> > + */
> > +# define VIR_DOMAIN_VCPU_STATE "state"
> > +
> > /**
> > * virDomainSetVcpuTuneParameters:
> > *
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index 772cb405d6..59d94c6dd6 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -20081,6 +20081,34 @@ qemuDomainGraphicsReload(virDomainPtr domain,
> > return ret;
> > }
> >
> > +static int
> > +qemuDomainSetVcpuTuneParameters(virDomainPtr dom,
> > + const char *vcpumap,
> > + virTypedParameterPtr params,
> > + int nparams,
> > + unsigned int flags)
> > +{
> > + int state;
> > + int rc;
> > +
> > + if (virTypedParamsValidate(params, nparams,
> > + VIR_DOMAIN_VCPU_STATE,
> > + VIR_TYPED_PARAM_INT,
> > + NULL) < 0)
> > + return -1;
> > +
> > + if ((rc = virTypedParamsGetInt(params, nparams,
> > + VIR_DOMAIN_VCPU_STATE,
> > + &state)) < 0)
> > + return -1;
> > +
> > + if (rc == 1) {
> > + return qemuDomainSetVcpu(dom, vcpumap, state, flags);
>
> I don't think we want to expose cpu hotplug via another API. Especially
> this has weird semantics once you try to combine it with other
> "parameter" setting.
>
> E.g. what if you set CPU to disabled, but set some other parameters?
>
> It has even ordering implications because enabling CPU needs to happen
> first.
>
> I don't think this should be done. CPU state doesn't even feel like a
> 'tunable' variable.
>
Yes, I also feel kind of weird.
What about the dirty rate limit(dirty-limit) for CPU, I think that is a
'tunable' variable.
>
> NACK
>
> > + }
> > +
> > + return 0;
> > +}
>
>
--
Best regards
