Re: [PATCH v4 2/3] qemu: extract common PCI handling functions

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

 



On 05/11/2014 08:48 AM, Roman Bogorodskiy wrote:
> Move sharable PCI handling functions to domain_addr.[ch], and
> change theirs prefix from 'qemu' to 'vir':
> 
>  - virDomainPCIAddressAsString;
>  - virDomainPCIAddressBusSetModel;
>  - virDomainPCIAddressEnsureAddr;
>  - virDomainPCIAddressFlagsCompatible;
>  - virDomainPCIAddressGetNextSlot;
>  - virDomainPCIAddressReleaseSlot;
>  - virDomainPCIAddressReserveAddr;
>  - virDomainPCIAddressReserveNextSlot;
>  - virDomainPCIAddressReserveSlot;
>  - virDomainPCIAddressSetFree;
>  - virDomainPCIAddressSetGrow;
>  - virDomainPCIAddressSlotInUse;
>  - virDomainPCIAddressValidate;
> 
> The only change here is function names, the implementation itself
> stays untouched.
> 
> Extract common allocation code from DomainPCIAddressSetCreate
> into virDomainPCIAddressSetAlloc.
> ---
>  po/POTFILES.in           |   1 +
>  src/conf/domain_addr.c   | 542 ++++++++++++++++++++++++++++++++++++++
>  src/conf/domain_addr.h   |  70 +++++
>  src/libvirt_private.syms |  17 ++
>  src/qemu/qemu_command.c  | 671 ++++++-----------------------------------------
>  src/qemu/qemu_command.h  |  18 +-
>  src/qemu/qemu_domain.c   |   3 +-
>  src/qemu/qemu_hotplug.c  |   8 +-
>  src/qemu/qemu_process.c  |   2 +-
>  9 files changed, 718 insertions(+), 614 deletions(-)
> 

> +
> +int
> +virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs,
> +                              virDomainDeviceInfoPtr dev)
> +{
> +    int ret = -1;
> +    char *addrStr = NULL;
> +    /* Flags should be set according to the particular device,
> +     * but only the caller knows the type of device. Currently this
> +     * function is only used for hot-plug, though, and hot-plug is
> +     * only supported for standard PCI devices, so we can safely use
> +     * the setting below */
> +    virDomainPCIConnectFlags flags = (VIR_PCI_CONNECT_HOTPLUGGABLE |
> +                                      VIR_PCI_CONNECT_TYPE_PCI);
> +
> +    if (!(addrStr = virDomainPCIAddressAsString(&dev->addr.pci)))
> +        goto cleanup;
> +
> +    if (dev->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
> +        /* We do not support hotplug multi-function PCI device now, so we should
> +         * reserve the whole slot. The function of the PCI device must be 0.
> +         */
> +        if (dev->addr.pci.function != 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("Only PCI device addresses with function=0"
> +                             " are supported"));
> +            goto cleanup;
> +        }
> +
> +        if (!virDomainPCIAddressValidate(addrs, &dev->addr.pci,
> +                                          addrStr, flags, true))

Indentation is off.

> +            goto cleanup;
> +
> +        ret = virDomainPCIAddressReserveSlot(addrs, &dev->addr.pci, flags);
> +    } else {
> +        ret = virDomainPCIAddressReserveNextSlot(addrs, dev, flags);
> +    }
> +
> + cleanup:
> +    VIR_FREE(addrStr);
> +    return ret;
> +}
> +

> diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
> index f5a5199..c59ef85 100644
> --- a/src/conf/domain_addr.h
> +++ b/src/conf/domain_addr.h
> @@ -76,4 +76,74 @@ typedef virDomainPCIAddressSet *virDomainPCIAddressSetPtr;

> +bool virDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr,
> +                                        const char *addrStr,
> +                                        virDomainPCIConnectFlags busFlags,
> +                                        virDomainPCIConnectFlags devFlags,
> +                                        bool reportError,
> +                                        bool fromConfig)
> +     ATTRIBUTE_NONNULL(1);
> +
> +bool virDomainPCIAddressValidate(virDomainPCIAddressSetPtr addrs,
> +                                 virDevicePCIAddressPtr addr,
> +                                 const char *addrStr,
> +                                 virDomainPCIConnectFlags flags,
> +                                 bool fromConfig)
> +     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);

For both of these, addrStr can be marked as ATTRIBUTE_NONNULL too.

> +
> +
> +int virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus,
> +                                   virDomainControllerModelPCI model)
> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);

model is an enum, 0 is a valid value.

> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index ebf17a8..aeaadbd 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c

> @@ -1240,7 +1240,7 @@ void qemuDomainCCWAddressSetFree(qemuDomainCCWAddressSetPtr addrs)
>  static qemuDomainCCWAddressSetPtr
>  qemuDomainCCWAddressSetCreate(void)
>  {
> -     qemuDomainCCWAddressSetPtr addrs = NULL;
> +    qemuDomainCCWAddressSetPtr addrs = NULL;
>  
>      if (VIR_ALLOC(addrs) < 0)
>          goto error;

Unrelated whitespace change.

ACK with the ATTRIBUTE_NONNULL removed for model.

Jan

Attachment: signature.asc
Description: OpenPGP digital signature

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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]