Re: [PATCH 2/4] vcpupin: implement the code to address the new API in the qemu driver

[Osier Yang <jyang@xxxxxxxxxx>]

ä 2011å03æ31æ 15:33, Osier Yang åé:
> ä 2011å03æ31æ 13:38, Taku Izumi åé:
> > This patch implements the code to address the new API
> > (virDomainPinVcpuFlags)
> > in the qemu driver.
> >
> > Signed-off-by: Taku Izumi<izumi.taku@xxxxxxxxxxxxxx>
> > ---
> > src/qemu/qemu_driver.c | 89
> > ++++++++++++++++++++++++++++++++++++-------------
> > 1 file changed, 67 insertions(+), 22 deletions(-)
> >
> > Index: libvirt/src/qemu/qemu_driver.c
> > ===================================================================
> > --- libvirt.orig/src/qemu/qemu_driver.c
> > +++ libvirt/src/qemu/qemu_driver.c
> > @@ -2624,17 +2624,29 @@ qemudDomainSetVcpus(virDomainPtr dom, un
> >
> >
> > static int
> > -qemudDomainPinVcpu(virDomainPtr dom,
> > - unsigned int vcpu,
> > - unsigned char *cpumap,
> > - int maplen) {
> > +qemudDomainPinVcpuFlags(virDomainPtr dom,
> > + unsigned int vcpu,
> > + unsigned char *cpumap,
> > + int maplen,
> > + unsigned int flags) {
> > +
> > struct qemud_driver *driver = dom->conn->privateData;
> > virDomainObjPtr vm;
> > + virDomainDefPtr persistentDef = NULL;
> > int maxcpu, hostcpus;
> > virNodeInfo nodeinfo;
> > int ret = -1;
> > qemuDomainObjPrivatePtr priv;
> >
> > + virCheckFlags(VIR_DOMAIN_VCPU_LIVE |
> > + VIR_DOMAIN_VCPU_CONFIG, -1);
> > +
> > + if ((flags& (VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_MEM_CONFIG)) == 0) {
>
> s/VIR_DOMAIN_MEM_CONFIG/VIR_DOMAIN_VCPU_CONFIG/
>
> This allows both "LIVE" and "CONFIG" are set, and you use two
> "if" clauses later for both of them, (
> if (flags& VIR_DOMAIN_VCPU_CONFIG), and
> if (flags& VIR_DOMAIN_VCPU_LIVE) ) so there should be problem.
>
> IMHO it should be: Exactly one of "LIVE" or "CONFIG" is set.

Or is it designed like so, allow both "LIVE" and "CONFIG" is set?

> > + qemuReportError(VIR_ERR_INVALID_ARG,
> > + _("invalid flag combination: (0x%x)"), flags);
> > + return -1;
> > + }
> > +
> > qemuDriverLock(driver);
> > vm = virDomainFindByUUID(&driver->domains, dom->uuid);
> > qemuDriverUnlock(driver);
> > @@ -2647,7 +2659,7 @@ qemudDomainPinVcpu(virDomainPtr dom,
> > goto cleanup;
> > }
> >
> > - if (!virDomainObjIsActive(vm)) {
> > + if (!virDomainObjIsActive(vm)&& (flags& VIR_DOMAIN_VCPU_LIVE)) {
> > qemuReportError(VIR_ERR_OPERATION_INVALID,
> > "%s",_("cannot pin vcpus on an inactive domain"));
> > goto cleanup;
> > @@ -2662,27 +2674,52 @@ qemudDomainPinVcpu(virDomainPtr dom,
> > goto cleanup;
> > }
> >
> > - if (nodeGetInfo(dom->conn,&nodeinfo)< 0)
> > - goto cleanup;
> > + if (flags& VIR_DOMAIN_VCPU_CONFIG) {
> > + if (!vm->persistent) {
> > + qemuReportError(VIR_ERR_OPERATION_INVALID, "%s",
> > + _("cannot change persistent config of a transient domain"));
> > + goto cleanup;
> > + }
> > + if (!(persistentDef = virDomainObjGetPersistentDef(driver->caps, vm)))
> > + goto cleanup;
> > + }
> >
> > - hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo);
> > - maxcpu = maplen * 8;
> > - if (maxcpu> hostcpus)
> > - maxcpu = hostcpus;
> > + if (flags& VIR_DOMAIN_VCPU_LIVE) {
> >
> > - if (priv->vcpupids != NULL) {
> > - if (virProcessInfoSetAffinity(priv->vcpupids[vcpu],
> > - cpumap, maplen, maxcpu)< 0)
> > + if (nodeGetInfo(dom->conn,&nodeinfo)< 0)
> > goto cleanup;
> > - } else {
> > - qemuReportError(VIR_ERR_NO_SUPPORT,
> > - "%s", _("cpu affinity is not supported"));
> > - goto cleanup;
> > +
> > + hostcpus = VIR_NODEINFO_MAXCPUS(nodeinfo);
> > + maxcpu = maplen * 8;
> > + if (maxcpu> hostcpus)
> > + maxcpu = hostcpus;
> > +
> > + if (priv->vcpupids != NULL) {
> > + if (virProcessInfoSetAffinity(priv->vcpupids[vcpu],
> > + cpumap, maplen, maxcpu)< 0)
> > + goto cleanup;
> > + } else {
> > + qemuReportError(VIR_ERR_NO_SUPPORT,
> > + "%s", _("cpu affinity is not supported"));
> > + goto cleanup;
> > + }
> > +
> > + if (virDomainVcpupinAdd(vm->def, cpumap, maplen, vcpu)< 0) {
> > + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > + _("failed to update or add vcpupin xml of a running domain"));
>
> Indention, :-)
>
> > + goto cleanup;
> > + }
> > +
> > }
> >
> > - if (virDomainVcpupinAdd(vm->def, cpumap, maplen, vcpu)< 0) {
> > - qemuReportError(VIR_ERR_INTERNAL_ERROR,
> > - "%s", _("failed to update or add vcpupin xml"));
> > + if (flags& VIR_DOMAIN_VCPU_CONFIG) {
> > +
> > + if (virDomainVcpupinAdd(persistentDef, cpumap, maplen, vcpu)< 0) {
> > + qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > + _("failed to update or add vcpupin xml of a persistent domain"));
>
> likewise
>
> > + goto cleanup;
> > + }
> > + ret = virDomainSaveConfig(driver->configDir, persistentDef);
> > goto cleanup;
> > }
> >
> > @@ -2695,6 +2732,14 @@ cleanup:
> > }
> >
> > static int
> > +qemudDomainPinVcpu(virDomainPtr dom,
> > + unsigned int vcpu,
> > + unsigned char *cpumap,
> > + int maplen) {
> > + return qemudDomainPinVcpuFlags(dom, vcpu, cpumap, maplen,
> > VIR_DOMAIN_VCPU_LIVE);
> > +}
> > +
> > +static int
> > qemudDomainGetVcpus(virDomainPtr dom,
> > virVcpuInfoPtr info,
> > int maxinfo,
> > @@ -6854,7 +6899,7 @@ static virDriver qemuDriver = {
> > qemudDomainSetVcpusFlags, /* domainSetVcpusFlags */
> > qemudDomainGetVcpusFlags, /* domainGetVcpusFlags */
> > qemudDomainPinVcpu, /* domainPinVcpu */
> > - NULL, /* domainPinVcpuFlags */
> > + qemudDomainPinVcpuFlags, /* domainPinVcpuFlags */
> > qemudDomainGetVcpus, /* domainGetVcpus */
> > qemudDomainGetMaxVcpus, /* domainGetMaxVcpus */
> > qemudDomainGetSecurityLabel, /* domainGetSecurityLabel */
> >
> >
> > --
> > libvir-list mailing list
> > libvir-list@xxxxxxxxxx
> > https://www.redhat.com/mailman/listinfo/libvir-list
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list