On 03/17/2015 07:41 AM, Ján Tomko wrote:
> Create a sorted array of virtio-serial controllers.
> Each of the elements contains the controller index
> and a bitmap of available ports.
>
> Buses are not tracked, because they aren't supported by QEMU.
> ---
> src/conf/domain_addr.c | 348 +++++++++++++++++++++++++++++++++++++++++++++++
> src/conf/domain_addr.h | 56 ++++++++
> src/libvirt_private.syms | 9 ++
> 3 files changed, 413 insertions(+)
>
I assumed the ACK to 1/5 sticks...
> diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
> index fb4a76f..d9d01fc 100644
> --- a/src/conf/domain_addr.c
> +++ b/src/conf/domain_addr.c
> @@ -718,3 +718,351 @@ virDomainCCWAddressSetCreate(void)
> virDomainCCWAddressSetFree(addrs);
> return NULL;
> }
> +
> +
> +#define VIR_DOMAIN_DEFAULT_VIRTIO_SERIAL_PORTS 31
> +
> +
> +/* virDomainVirtioSerialAddrSetCreate
> + *
> + * Allocates an address set for virtio serial addresses
> + */
> +virDomainVirtioSerialAddrSetPtr
> +virDomainVirtioSerialAddrSetCreate(void)
> +{
> + virDomainVirtioSerialAddrSetPtr ret = NULL;
> +
> + if (VIR_ALLOC(ret) < 0)
> + return NULL;
> +
> + return ret;
> +}
> +
> +static void
> +virDomainVirtioSerialControllerFree(virDomainVirtioSerialControllerPtr cont)
Should the Free routine take a pointer so that when we VIR_FREE the
pointer the caller doesn't have to "worry" about setting their copy to NULL?
> +{
> + if (cont) {
> + virBitmapFree(cont->ports);
> + VIR_FREE(cont);
> + }
> +}
> +
> +static ssize_t
> +virDomainVirtioSerialAddrPlaceController(virDomainVirtioSerialAddrSetPtr addrs,
> + virDomainVirtioSerialControllerPtr cont)
> +{
> + size_t i;
> +
> + for (i = 0; i < addrs->ncontrollers; i++) {
> + if (addrs->controllers[i]->idx >= cont->idx)
> + return i;
> + }
Would entries "<controller type='virtio-serial' index='1' ports='4'>"
and "<controller type='virtio-serial' index='1'"> be rejected elsewhere?
I would think "index" would be unique but this algorithm seems to be
fine and happy with it.
> + return -1;
> +}
> +
> +static ssize_t
> +virDomainVirtioSerialAddrFindController(virDomainVirtioSerialAddrSetPtr addrs,
> + unsigned int idx)
> +{
> + size_t i;
> +
> + for (i = 0; i < addrs->ncontrollers; i++) {
> + if (addrs->controllers[i]->idx == idx)
> + return i;
> + }
> + return -1;
> +}
> +
> +/* virDomainVirtioSerialAddrSetAddController
> + *
> + * Adds virtio serial ports of the existing controller
> + * to the address set.
> + */
> +int
> +virDomainVirtioSerialAddrSetAddController(virDomainVirtioSerialAddrSetPtr addrs,
> + virDomainControllerDefPtr cont)
> +{
> + int ret = -1;
> + int ports;
> + virDomainVirtioSerialControllerPtr cnt = NULL;
> + ssize_t insertAt;
> +
> + if (cont->type != VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL)
> + return 0;
> +
> + ports = cont->opts.vioserial.ports;
> + if (ports == -1)
> + ports = VIR_DOMAIN_DEFAULT_VIRTIO_SERIAL_PORTS;
> +
> + VIR_DEBUG("Adding virtio serial controller index %u with %d"
> + " ports to the address set", cont->idx, ports);
> +
> + if (VIR_ALLOC(cnt) < 0)
> + goto cleanup;
> +
> + if (!(cnt->ports = virBitmapNew(ports)))
> + goto cleanup;
> + cnt->idx = cont->idx;
> +
> + insertAt = virDomainVirtioSerialAddrPlaceController(addrs, cnt);
> + if (VIR_INSERT_ELEMENT(addrs->controllers, insertAt,
> + addrs->ncontrollers, cnt) < 0)
> + goto cleanup;
> +
> + ret = 0;
> +
> + cleanup:
> + virDomainVirtioSerialControllerFree(cnt);
> + return ret;
> +}
> +
> +/* virDomainVirtioSerialAddrSetAddControllers
> + *
> + * Adds virtio serial ports of controllers present in the domain definition
> + * to the address set.
> + */
> +int
> +virDomainVirtioSerialAddrSetAddControllers(virDomainVirtioSerialAddrSetPtr addrs,
> + virDomainDefPtr def)
> +{
> + size_t i;
> +
> + for (i = 0; i < def->ncontrollers; i++) {
> + if (virDomainVirtioSerialAddrSetAddController(addrs,
> + def->controllers[i]) < 0)
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +/* virDomainVirtioSerialAddrSetRemoveController
> + *
> + * Removes a virtio serial controller from the address set.
> + */
> +int
> +virDomainVirtioSerialAddrSetRemoveController(virDomainVirtioSerialAddrSetPtr addrs,
> + virDomainControllerDefPtr cont)
> +{
> + int ret = -1;
> + ssize_t pos;
> +
> + if (cont->type != VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL)
> + return 0;
> +
> + VIR_DEBUG("Removing virtio serial controller index %u "
> + "from the address set", cont->idx);
> +
> + pos = virDomainVirtioSerialAddrFindController(addrs, cont->idx);
> +
> + if (pos >= 0 &&
> + VIR_DELETE_ELEMENT(addrs->controllers, pos, addrs->ncontrollers) < 0)
> + goto cleanup;
> +
If 'pos' < 0, we return 0 (and perhaps leak something). OTOH, the
controller was never added and the caller never checks status, maybe
this should just be void (wonder why Coverity didn't whine)...
> + ret = 0;
> +
> + cleanup:
> + return ret;
> +}
> +
> +void
> +virDomainVirtioSerialAddrSetFree(virDomainVirtioSerialAddrSetPtr addrs)
> +{
same question about having Free routine take address rather than refcopy
> + size_t i;
> + if (addrs) {
> + for (i = 0; i < addrs->ncontrollers; i++)
> + virDomainVirtioSerialControllerFree(addrs->controllers[i]);
> + VIR_FREE(addrs);
> + }
> +}
> +
> +static int
> +virDomainVirtioSerialAddrNext(virDomainVirtioSerialAddrSetPtr addrs,
> + virDomainDeviceVirtioSerialAddress *addr,
> + bool allowZero)
> +{
> + int ret = -1;
> + ssize_t port, start = 0;
> + ssize_t i;
> + unsigned int controller;
> +
> + /* port number 0 is reserved for virtconsoles */
> + if (allowZero)
> + start = -1;
> +
> + if (addrs->ncontrollers == 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("no virtio-serial controllers are available"));
> + goto cleanup;
> + }
> +
> + for (i = 0; i < addrs->ncontrollers; i++) {
> + virBitmapPtr map = addrs->controllers[i]->ports;
> + if ((port = virBitmapNextClearBit(map, start)) >= 0) {
> + controller = addrs->controllers[i]->idx;
> + goto success;
> + }
> + }
> +
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("Unable to find a free virtio-serial port"));
> +
> + cleanup:
> + return ret;
> +
> + success:
> + addr->bus = 0;
> + addr->port = port;
> + addr->controller = controller;
> + VIR_DEBUG("Found free virtio serial controller %u port %u", addr->controller,
> + addr->port);
> + ret = 0;
> + goto cleanup;
> +}
> +
> +/* virDomainVirtioSerialAddrAutoAssign
> + *
> + * reserve a virtio serial address of the device (if it has one)
> + * or assign a virtio serial address to the device
> + */
> +int
> +virDomainVirtioSerialAddrAutoAssign(virDomainVirtioSerialAddrSetPtr addrs,
> + virDomainDeviceInfoPtr info,
> + bool allowZero)
> +{
> + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL &&
> + info->addr.vioserial.port)
> + return virDomainVirtioSerialAddrReserve(NULL, NULL, info, addrs);
> + else
> + return virDomainVirtioSerialAddrAssign(addrs, info, allowZero);
> +}
> +
> +
> +int
> +virDomainVirtioSerialAddrAssign(virDomainVirtioSerialAddrSetPtr addrs,
> + virDomainDeviceInfoPtr info,
> + bool allowZero)
> +{
> + int ret = -1;
> + virDomainDeviceInfo nfo = { NULL };
> + virDomainDeviceInfoPtr ptr = allowZero ? &nfo : info;
> +
> + ptr->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL;
> +
> + if (virDomainVirtioSerialAddrNext(addrs, &ptr->addr.vioserial,
> + allowZero) < 0)
> + goto cleanup;
> +
> + if (virDomainVirtioSerialAddrReserve(NULL, NULL, ptr, addrs) < 0)
> + goto cleanup;
> +
> + ret = 0;
> +
> + cleanup:
> + return ret;
> +}
> +
> +/* virDomainVirtioSerialAddrReserve
> + *
> + * Reserve the virtio serial address of the device
> + *
> + * For use with virDomainDeviceInfoIterate,
> + * opaque should be the address set
> + */
> +int
> +virDomainVirtioSerialAddrReserve(virDomainDefPtr def ATTRIBUTE_UNUSED,
> + virDomainDeviceDefPtr dev ATTRIBUTE_UNUSED,
> + virDomainDeviceInfoPtr info,
> + void *data)
> +{
> + virDomainVirtioSerialAddrSetPtr addrs = data;
> + char *str = NULL;
> + int ret = -1;
> + virBitmapPtr map = NULL;
> + bool b;
> + ssize_t i;
> +
> + if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL ||
> + info->addr.vioserial.port == 0)
> + return 0;
> +
> + VIR_DEBUG("Reserving virtio serial %u %u", info->addr.vioserial.controller,
> + info->addr.vioserial.port);
> +
> + i = virDomainVirtioSerialAddrFindController(addrs, info->addr.vioserial.controller);
> + if (i < 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("virtio serial controller %u is missing"),
> + info->addr.vioserial.controller);
> + goto cleanup;
> + }
> +
> + map = addrs->controllers[i]->ports;
> + if (virBitmapGetBit(map, info->addr.vioserial.port, &b) < 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("virtio serial controller %u does not have port %u"),
> + info->addr.vioserial.controller,
> + info->addr.vioserial.port);
> + goto cleanup;
> + }
> +
> + if (b) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("virtio serial port %u on controller %u is already occupied"),
> + info->addr.vioserial.port,
> + info->addr.vioserial.controller);
> + goto cleanup;
> + }
> +
> + ignore_value(virBitmapSetBit(map, info->addr.vioserial.port));
> +
> + ret = 0;
> +
> + cleanup:
> + VIR_FREE(str);
> + return ret;
> +}
> +
> +/* virDomainVirtioSerialAddrRelease
> + *
> + * Release the virtio serial address of the device
> + */
> +int
> +virDomainVirtioSerialAddrRelease(virDomainVirtioSerialAddrSetPtr addrs,
> + virDomainDeviceInfoPtr info)
> +{
> + virBitmapPtr map;
> + char *str = NULL;
> + int ret = -1;
> + ssize_t i;
> +
> + if (info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_SERIAL ||
> + info->addr.vioserial.port == 0)
> + return 0;
> +
> + VIR_DEBUG("Releasing virtio serial %u %u", info->addr.vioserial.controller,
> + info->addr.vioserial.port);
> +
> + i = virDomainVirtioSerialAddrFindController(addrs, info->addr.vioserial.controller);
> + if (i < 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("virtio serial controller %u is missing"),
> + info->addr.vioserial.controller);
> + goto cleanup;
> + }
> +
> + map = addrs->controllers[i]->ports;
> + if (virBitmapClearBit(map, info->addr.vioserial.port) < 0) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("virtio serial controller %u does not have port %u"),
> + info->addr.vioserial.controller,
> + info->addr.vioserial.port);
> + goto cleanup;
> + }
Should we info->addr.vioserial.port = 0 here to ensure someone doesn't
end up in some loop retrying the same port?
This seems reasonable; however, results in next patch having me
scratching my head a bit...
John
> +
> + ret = 0;
> +
> + cleanup:
> + VIR_FREE(str);
> + return ret;
> +}
> diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
> index 2c3468e..846bf5c 100644
> --- a/src/conf/domain_addr.h
> +++ b/src/conf/domain_addr.h
> @@ -173,4 +173,60 @@ int virDomainCCWAddressReleaseAddr(virDomainCCWAddressSetPtr addrs,
> virDomainDeviceInfoPtr dev)
> ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
> virDomainCCWAddressSetPtr virDomainCCWAddressSetCreate(void);
> +
> +struct _virDomainVirtioSerialController {
> + unsigned int idx;
> + virBitmapPtr ports;
> +};
> +
> +typedef struct _virDomainVirtioSerialController virDomainVirtioSerialController;
> +typedef virDomainVirtioSerialController *virDomainVirtioSerialControllerPtr;
> +
> +struct _virDomainVirtioSerialAddrSet {
> + virDomainVirtioSerialControllerPtr *controllers;
> + size_t ncontrollers;
> +};
> +typedef struct _virDomainVirtioSerialAddrSet virDomainVirtioSerialAddrSet;
> +typedef virDomainVirtioSerialAddrSet *virDomainVirtioSerialAddrSetPtr;
> +
> +virDomainVirtioSerialAddrSetPtr
> +virDomainVirtioSerialAddrSetCreate(void);
> +int
> +virDomainVirtioSerialAddrSetAddController(virDomainVirtioSerialAddrSetPtr addrs,
> + virDomainControllerDefPtr cont)
> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
> +int
> +virDomainVirtioSerialAddrSetRemoveController(virDomainVirtioSerialAddrSetPtr addrs,
> + virDomainControllerDefPtr cont)
> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
> +int
> +virDomainVirtioSerialAddrSetAddControllers(virDomainVirtioSerialAddrSetPtr addrs,
> + virDomainDefPtr def)
> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
> +void
> +virDomainVirtioSerialAddrSetFree(virDomainVirtioSerialAddrSetPtr addrs);
> +int
> +virDomainVirtioSerialAddrAutoAssign(virDomainVirtioSerialAddrSetPtr addrs,
> + virDomainDeviceInfoPtr info,
> + bool allowZero)
> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
> +
> +int
> +virDomainVirtioSerialAddrAssign(virDomainVirtioSerialAddrSetPtr addrs,
> + virDomainDeviceInfoPtr info,
> + bool allowZero)
> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
> +
> +int
> +virDomainVirtioSerialAddrReserve(virDomainDefPtr def,
> + virDomainDeviceDefPtr dev,
> + virDomainDeviceInfoPtr info,
> + void *data)
> + ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
> +
> +int
> +virDomainVirtioSerialAddrRelease(virDomainVirtioSerialAddrSetPtr addrs,
> + virDomainDeviceInfoPtr info)
> + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
> +
> #endif /* __DOMAIN_ADDR_H__ */
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 1fb42ac..f600838 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -108,6 +108,15 @@ virDomainPCIAddressSetFree;
> virDomainPCIAddressSetGrow;
> virDomainPCIAddressSlotInUse;
> virDomainPCIAddressValidate;
> +virDomainVirtioSerialAddrAssign;
> +virDomainVirtioSerialAddrAutoAssign;
> +virDomainVirtioSerialAddrRelease;
> +virDomainVirtioSerialAddrReserve;
> +virDomainVirtioSerialAddrSetAddController;
> +virDomainVirtioSerialAddrSetAddControllers;
> +virDomainVirtioSerialAddrSetCreate;
> +virDomainVirtioSerialAddrSetFree;
> +virDomainVirtioSerialAddrSetRemoveController;
>
>
> # conf/domain_audit.h
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list