Re: [Qemu-devel] [RFC PATCH 1/5] qdev: Create qdev_get_dev_path()

[Alex Williamson <alex.williamson@redhat.com>]

On Mon, 2010-06-14 at 14:09 +0100, Paul Brook wrote:
> > > > "/main-system-bus/pci.0,addr=09.0/virtio-blk-pci"
> 
> There's a device missing between the main system bus and the pci bus.  Should 
> be something like:
> 
> /main-system-bus/piix4-pcihost/pci.0/_09.0

Ok, I can easily come up with:

/System/main-system-bus/i440FX-pcihost/PCI/pci.0/_09.0/virtio-blk-pci/virtio-blk

to expand the previous output.  Code below.

> > > Could you explain why you add "identified properties of the immediate
> > > parent bus and device"?  They make the result ver much *not* a "dev
> > > path" in the qdev sense...
> > 
> > In order to try to get a unique string.  Without looking into device
> > properties, two e1000s would both be:
> > 
> > /main-system-bus/pci.0/e1000
> > /main-system-bus/pci.0/e1000
> > 
> > Which is no better than simply "e1000" and would require us to fall back
> > to instance ids again.  The goal here is that anything that makes use of
> > passing a dev when registering a vmstate gets an instance id of zero.
> 
> You already got the information you need, you just put it in the wrong place. 
> The canonical ID for the device could be its bus address. In practice we'd 
> probably want to allow the user to specify it by name, provided these are 
> unique. e.g. in the above machine we could accept [...]/virtiio-blk-pci would 
> as an aias for [...]:_09.0.

Sure, if there was a guaranteed unique char* on a DeviceState that was
consistent between guest invocations, we could just use that instead.  I
suppose all devices could have a global system id property and if that
was present we'd use that instead of creating the device path.

> Device names have a restricted namespace, so we 
> can use an initial prefix to disambiguate a name/label from a bus address.

I'm not sure it's necessary.  It seems like it would only be necessary
to differentiate the two if we wanted to translate between namespaces.
But I think it's reasonable to require that if a global name is
provided, it must always be provided for the life of the VM.

> For busses that don't have a consistent addressing scheme then some sort of 
> instance ID is unavoidable. I guess it may be possible to invent something 
> based on other device properties (e.g. address of the first IO port/memory 
> region).

Instance IDs aren't always bad, we just run into trouble with hotplug
and maybe creating unique ramblock names.  But, such busses typically
don't support hotplug and don't have multiple instances of the same
device on the bus, so I don't expect us to hit many issues there as long
as we can get a stable address path.  Thanks,

Alex

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---

static int qdev_strprint_path_props(DeviceState *dev, Property *props,
                                    char *buf, size_t len)
{
    int offset = 0;
    char pbuf[64];

    if (!props)
        return 0;

    while (props->name) {
        if (props->info->flags & PROP_FLAG_PATH) {
            if (props->info->print) {
                props->info->print(dev, props, pbuf, sizeof(pbuf));
                offset += snprintf(buf + offset, len - offset, "/_%s", pbuf);
            }
        }
        props++;
    }
    return offset;
}

static int qdev_strprint_dev(DeviceState *dev, char *buf, size_t len);

static int qdev_strprint_bus(DeviceState *dev, char *buf, size_t len)
{
    int offset = 0;

    if (dev->parent_bus->parent)
        offset += qdev_strprint_dev(dev->parent_bus->parent, buf, len);

    offset += snprintf(buf + offset, len - offset, "/%s/%s",
                       dev->parent_bus->info->name, dev->parent_bus->name);

    offset += qdev_strprint_path_props(dev, dev->parent_bus->info->props,
                                       buf + offset, len - offset);

    return offset;
}

static int qdev_strprint_dev(DeviceState *dev, char *buf, size_t len)
{
    int offset = 0;

    if (dev->parent_bus)
        offset += qdev_strprint_bus(dev, buf, len);

    offset += snprintf(buf + offset, len - offset, "/%s", dev->info->name);

    offset += qdev_strprint_path_props(dev, dev->info->props,
                                       buf + offset, len - offset);
    if (dev->id)
        offset += snprintf(buf + offset, len - offset, "/%s", dev->id);

    return offset;

}

char *qdev_get_dev_path(DeviceState *dev)
{
    char buf[256] = "";

    if (!dev)
        return NULL;

    qdev_strprint_dev(dev, buf, sizeof(buf));
    return qemu_strdup(buf);
}