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

Re: [libvirt] [PATCH 6/6] qemu: Add spapr-vio address assignment



On 12/07/2011 11:41 PM, Michael Ellerman wrote:
> From: Michael Ellerman <michael ellerman id au>
> 
> Add logic to assign addresses for devices with spapr-vio addresses.
> 
> We also do validation of addresses specified by the user, ie. ensuring
> that there are not duplicate addresses on the bus.
> 
> Signed-off-by: Michael Ellerman <michael ellerman id au>
> ---
>  src/qemu/qemu_command.c |   87 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 87 insertions(+), 0 deletions(-)

Depends on patch 4, but I'll review anyway...

> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 32ee8d8..62a1258 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -684,6 +684,87 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virBitmapPtr qemuCaps)
>      return -1;
>  }
>  
> +static int
> +qemuSpaprVIOFindByReg(virDomainDefPtr def ATTRIBUTE_UNUSED,
> +                      virDomainDeviceInfoPtr info, void *opaque)
> +{
> +    virDomainDeviceInfoPtr target = (virDomainDeviceInfoPtr)opaque;

The cast isn't strictly necessary in C.

> +
> +    if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO)
> +        return 0;
> +
> +    /* Match a dev that has a reg, is not us, and has a matching reg */
> +    if (info->addr.spaprvio.has_reg && info != target &&

As I asked in 4, can a valid reg ever be 0, or must it be non-zero?  If
the latter, then you don't need has_reg.

> +        info->addr.spaprvio.reg == target->addr.spaprvio.reg)
> +        /* Has to be < 0 so virDomainDeviceInfoIterate() will exit */
> +        return -1;
> +
> +    return 0;
> +}
> +
> +static int
> +qemuAssignSpaprVIOAddress(virDomainDefPtr def, virDomainDeviceInfoPtr info,
> +                          unsigned long long default_reg)
> +{
> +    bool user_reg;
> +    int rc;
> +
> +    if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO)
> +        return 0;
> +
> +    /* Check if the user has assigned the reg already, if so use it */
> +    user_reg = info->addr.spaprvio.has_reg;
> +    if (!user_reg) {
> +        info->addr.spaprvio.reg = default_reg;
> +        info->addr.spaprvio.has_reg = true;
> +    }
> +
> +    rc = virDomainDeviceInfoIterate(def, qemuSpaprVIOFindByReg, info);
> +    while (rc != 0) {
> +        if (user_reg) {
> +            qemuReportError(VIR_ERR_XML_ERROR,
> +                            _("spapr-vio address %#llx already in use"),
> +                            info->addr.spaprvio.reg);
> +            return -EEXIST;
> +        }
> +
> +        /* We assigned the reg, so try a new value */
> +        info->addr.spaprvio.reg += 0x1000;
> +        rc = virDomainDeviceInfoIterate(def, qemuSpaprVIOFindByReg, info);
> +    }

Looks reasonable at either honoring the user's choice, or at iterating
by 0x1000 until a free slot is found.  Does that match the qemu algorithm?

> +
> +    return 0;
> +}
> +
> +static int qemuDomainAssignSpaprVIOAddresses(virDomainDefPtr def)
> +{
> +    int i, rc;
> +
> +    for (i = 0 ; i < def->nnets; i++) {
> +        rc = qemuAssignSpaprVIOAddress(def, &def->nets[i]->info,
> +                                       0x1000ul);
> +        if (rc)
> +            return rc;
> +    }
> +
> +    for (i = 0 ; i < def->ncontrollers; i++) {
> +        rc = qemuAssignSpaprVIOAddress(def, &def->controllers[i]->info,
> +                                       0x2000ul);
> +        if (rc)
> +            return rc;
> +    }
> +
> +    for (i = 0 ; i < def->nserials; i++) {
> +        rc = qemuAssignSpaprVIOAddress(def, &def->serials[i]->info,
> +                                       0x30000000ul);

Again, I assume these three defaults match qemu.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


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