[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libvir] [PATCH] Implement SetVcpus and DomainGetMaxVcpus for qemu



Cole Robinson <crobinso redhat com> wrote:
> The attached patch fills in two of the vcpu functions for the qemu driver:
>
> virDomainSetVcpus : set the number of vcpus the domain can use
> virDomainGetMaxVcpus : max number of vcpus that can be assigned to the domain.
>
> Code change is only in qemu_driver, as the backend stuff was already in place.
> I also edited qemudGetMaxVcpus to ignore case when checking the passed OS
> type, since it wasn't matching the returned results of qemudDomainGetOSType.

Hi Cole,
This looks fine, modulo a couple nits:
...
> diff --git a/src/qemu_driver.c b/src/qemu_driver.c
...
> +static int qemudDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus) {
> +    struct qemud_driver *driver = (struct qemud_driver *)dom->conn->privateData;

Since it doesn't affect an interface, it's less important,
but I think "driver" can be a const pointer.

> +    struct qemud_vm *vm = qemudFindVMByUUID(driver, dom->uuid);
> +    int max;
> +
> +    if (!vm) {
> +        qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_DOMAIN,
> +                         _("no domain with matching uuid '%s'"), dom->uuid);
> +        return -1;
> +    }
> +
> +    if (qemudIsActiveVM(vm)) {
> +        qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR, "%s",
> +                         _("cannot change vcpu count of an active domain"));
> +        return -1;
> +    }
> +
> +    if ((max = qemudDomainGetMaxVcpus(dom)) < 0) {
> +        qemudReportError(dom->conn, dom, NULL, VIR_ERR_INTERNAL_ERROR,

Please insert a "%s" argument here, like above, since
the message contains no "%"-directive.  This will avoid a
compile-time warning.

> +                         _("could not determine max vcpus for the domain"));
> +        return -1;
> +    }
> +
> +    if (nvcpus > max) {
> +        qemudReportError(dom->conn, dom, NULL, VIR_ERR_INVALID_ARG,
> +                         _("requested vcpus is greater than max allowable"
> +                           " vcpus for the domain: %d > %d"), nvcpus, max);
> +        return -1;
> +    }
> +
> +    vm->def->vcpus = nvcpus;
> +    return 0;
> +}


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]