Re: [PATCH] virtio-pci: add softlinks between virtio and pci

Anthony Liguori
2011-01-05T20:08:34Z

On 01/05/2011 02:05 PM, Michael S. Tsirkin wrote:
> On Wed, Jan 05, 2011 at 01:28:34PM -0600, Anthony Liguori wrote:
>
>> On 01/05/2011 01:17 PM, Michael S. Tsirkin wrote:
>>
>>> We sometimes need to map between the virtio device and
>>> the given pci device. One such use is OS installer that
>>> gets the boot pci device from BIOS and needs to
>>> find the relevant block device. Since it can't,
>>> installation fails.
>>>
>> I have no objection to this patch but I'm a tad confused by the description.
>>
>> I assume you mean the installer is querying the boot device via
>> int13 get driver parameters such that it returns the pci address of
>> the device?
>>
>> Or is it querying geometry information and then trying to find the
>> best match block device?
>>
>> If it's the former, I don't really understand the need for a
>> backlink since the PCI address gives you a link to the block device.
>> OTOH, if it's the later, it would make sense but then your
>> description doesn't really make much sense.
>>
>> At any rate, a better commit message would be helpful in explaining
>> the need for this.
>>
>> Regards,
>>
>> Anthony Liguori
>>
> OK just to clarify: we get pci address from BIOS
> and need the virtio device to get at the linux device
> (e.g. block) in the end. Thus the link from pci to virtio.
> I also added a backlink since I thought it's handy.
>
> Does this answer the questions?
>
> Rusty rewrites my commit logs anyway, he has better style :)
>

It helps. The real reason this is needed is because in a normal device,
there is only one struct device whereas with virtio-pci, the virtio-pci
device has a struct device and then the actual virtio device has another
one. There's probably a better way to handle this in sysfs making
virtio-pci a proper bus with only a single device as a child or
something like that.

But the links are probably an easier solution.

Regards,

Anthony Liguori

>
>>> Supply softlinks between these to make it possible.
>>>
>>> Signed-off-by: Michael S. Tsirkin<mst@redhat.com>
>>> ---
>>>
>>> Gleb, could you please ack that this patch below
>>> will be enough to fix the installer issue that
>>> you see?
>>>
>>> drivers/virtio/virtio_pci.c | 18 +++++++++++++++++-
>>> 1 files changed, 17 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
>>> index ef8d9d5..06eb2f8 100644
>>> --- a/drivers/virtio/virtio_pci.c
>>> +++ b/drivers/virtio/virtio_pci.c
>>> @@ -25,6 +25,7 @@
>>> #include<linux/virtio_pci.h>
>>> #include<linux/highmem.h>
>>> #include<linux/spinlock.h>
>>> +#include<linux/sysfs.h>
>>>
>>> MODULE_AUTHOR("Anthony Liguori<aliguori@us.ibm.com>");
>>> MODULE_DESCRIPTION("virtio-pci");
>>> @@ -667,8 +668,21 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
>>> if (err)
>>> goto out_set_drvdata;
>>>
>>> - return 0;
>>> + err = sysfs_create_link(&pci_dev->dev.kobj,&vp_dev->vdev.dev.kobj,
>>> + "virtio_device");
>>> + if (err)
>>> + goto out_register_device;
>>> +
>>> + err = sysfs_create_link(&vp_dev->vdev.dev.kobj,&pci_dev->dev.kobj,
>>> + "bus_device");
>>> + if (err)
>>> + goto out_create_link;
>>>
>>> + return 0;
>>> +out_create_link:
>>> + sysfs_remove_link(&pci_dev->dev.kobj, "virtio_device");
>>> +out_register_device:
>>> + unregister_virtio_device(&vp_dev->vdev);
>>> out_set_drvdata:
>>> pci_set_drvdata(pci_dev, NULL);
>>> pci_iounmap(pci_dev, vp_dev->ioaddr);
>>> @@ -685,6 +699,8 @@ static void __devexit virtio_pci_remove(struct pci_dev *pci_dev)
>>> {
>>> struct virtio_pci_device *vp_dev = pci_get_drvdata(pci_dev);
>>>
>>> + sysfs_remove_link(&vp_dev->vdev.dev.kobj, "bus_device");
>>> + sysfs_remove_link(&pci_dev->dev.kobj, "virtio_device");
>>> unregister_virtio_device(&vp_dev->vdev);
>>> }
>>>
>>>