virtio-pci: add softlinks between virtio and pci

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.

Supply softlinks between these to make it possible.

Signed-off-by Michael S. Tsirkin<[email protected]>

---

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(-)

drivers/virtio/virtio_pci.c +17 -1

index ef8d9d5..06eb2f8 100644
@@ -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<[email protected]>");
  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);
  }
 

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.

Supply softlinks between these to make it possible.

Signed-off-by Michael S. Tsirkin<[email protected]>

---

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(-)

drivers/virtio/virtio_pci.c +17 -1

index ef8d9d5..06eb2f8 100644
@@ -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<[email protected]>");
  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);
  }
 

Quoted 5 lines · Michael S. Tsirkin

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

Quoted 56 lines

Supply softlinks between these to make it possible.

Signed-off-by Michael S. Tsirkin<[email protected]>

---

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(-)

drivers/virtio/virtio_pci.c +17 -1

index ef8d9d5..06eb2f8 100644
@@ -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<[email protected]>");
 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);
 }
 

Quoted 5 lines · Michael S. Tsirkin

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

Quoted 56 lines

Supply softlinks between these to make it possible.

Signed-off-by Michael S. Tsirkin<[email protected]>

---

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(-)

drivers/virtio/virtio_pci.c +17 -1

index ef8d9d5..06eb2f8 100644
@@ -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<[email protected]>");
 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);
 }
 

Quoted 8 lines · Anthony Liguori

Quoted 6 lines · Michael S. Tsirkin

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.

Quoted 9 lines

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?

I think it's the former.

Quoted 3 lines

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.

It does? How does it?

Quoted 69 lines

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

Quoted 57 lines

Supply softlinks between these to make it possible.

Signed-off-by Michael S. Tsirkin<[email protected]>

---

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(-)

drivers/virtio/virtio_pci.c +17 -1

index ef8d9d5..06eb2f8 100644
@@ -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<[email protected]>");
  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);
  }
 

Quoted 8 lines · Anthony Liguori

Quoted 6 lines · Michael S. Tsirkin

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.

Quoted 9 lines

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?

I think it's the former.

Quoted 3 lines

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.

It does? How does it?

Quoted 69 lines

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

Quoted 57 lines

Supply softlinks between these to make it possible.

Signed-off-by Michael S. Tsirkin<[email protected]>

---

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(-)

drivers/virtio/virtio_pci.c +17 -1

index ef8d9d5..06eb2f8 100644
@@ -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<[email protected]>");
  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);
  }
 

Quoted 5 lines · Michael S. Tsirkin

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

Quoted 56 lines

Supply softlinks between these to make it possible.

Signed-off-by Michael S. Tsirkin<[email protected]>

---

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(-)

drivers/virtio/virtio_pci.c +17 -1

index ef8d9d5..06eb2f8 100644
@@ -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<[email protected]>");
 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);
 }
 

Quoted 5 lines · Michael S. Tsirkin

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

Quoted 56 lines

Supply softlinks between these to make it possible.

Signed-off-by Michael S. Tsirkin<[email protected]>

---

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(-)

drivers/virtio/virtio_pci.c +17 -1

index ef8d9d5..06eb2f8 100644
@@ -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<[email protected]>");
 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);
 }
 

Quoted 22 lines · Anthony Liguori

Quoted 6 lines · Michael S. Tsirkin

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.

Quoted 7 lines

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 :)

Quoted 58 lines

Quoted 58 lines

Supply softlinks between these to make it possible.

Signed-off-by Michael S. Tsirkin<[email protected]>

---

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(-)

drivers/virtio/virtio_pci.c +17 -1

index ef8d9d5..06eb2f8 100644
@@ -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<[email protected]>");
  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);
  }
 

Quoted 22 lines · Anthony Liguori

Quoted 6 lines · Michael S. Tsirkin

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.

Quoted 7 lines

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 :)

Quoted 58 lines

Quoted 58 lines

Supply softlinks between these to make it possible.

Signed-off-by Michael S. Tsirkin<[email protected]>

---

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(-)

drivers/virtio/virtio_pci.c +17 -1

index ef8d9d5..06eb2f8 100644
@@ -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<[email protected]>");
  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);
  }
 

Quoted 32 lines · Michael S. Tsirkin

Quoted 20 lines · Anthony Liguori

Quoted 5 lines · Michael S. Tsirkin

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.

Quoted 6 lines

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.

Quoted 3 lines

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

Quoted 56 lines

Quoted 56 lines

Quoted 56 lines

Supply softlinks between these to make it possible.

Signed-off-by Michael S. Tsirkin<[email protected]>

---

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(-)

drivers/virtio/virtio_pci.c +17 -1

index ef8d9d5..06eb2f8 100644
@@ -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<[email protected]>");
 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);
 }
 

Quoted 32 lines · Michael S. Tsirkin

Quoted 20 lines · Anthony Liguori

Quoted 5 lines · Michael S. Tsirkin

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.

Quoted 6 lines

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.

Quoted 3 lines

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

Quoted 56 lines

Quoted 56 lines

Quoted 56 lines

Supply softlinks between these to make it possible.

Signed-off-by Michael S. Tsirkin<[email protected]>

---

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(-)

drivers/virtio/virtio_pci.c +17 -1

index ef8d9d5..06eb2f8 100644
@@ -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<[email protected]>");
 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);
 }
 

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.

Supply softlinks between these to make it possible.

Signed-off-by Michael S. Tsirkin <[email protected]>

---

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(-)

drivers/virtio/virtio_pci.c +17 -1

index ef8d9d5..06eb2f8 100644
@@ -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 <[email protected]>");
 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);
 }
 

Signature · 1 line

1.7.3.2.91.g446ac

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.

Supply softlinks between these to make it possible.

Signed-off-by Michael S. Tsirkin <[email protected]>

---

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(-)

drivers/virtio/virtio_pci.c +17 -1

index ef8d9d5..06eb2f8 100644
@@ -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 <[email protected]>");
 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);
 }
 

Signature · 1 line

1.7.3.2.91.g446ac

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.

Supply softlinks between these to make it possible.

Gleb, could you please ack that this patch below will be enough to fix the installer issue that you see?

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.

Supply softlinks between these to make it possible.

Gleb, could you please ack that this patch below will be enough to fix the installer issue that you see?

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.

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Signed-off-by Milton Miller <[email protected]>

---

This is an alternative to the patch by Michael S. Tsirkin titled "virtio-pci: add softlinks between virtio and pci"
https://patchwork.kernel.org/patch/454581/

It creates simpler code, uses less memory, and should be even easier use by the installer as it won't have to know a virtio symlink to follow (just follow none).

Compile tested only as I don't have kvm setup.

drivers/virtio/virtio_pci.c +2 -18

index ef8d9d5..4fb5b2b 100644
@@ -96,11 +96,6 @@ static struct pci_device_id virtio_pci_id_table[] = {
 
 MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
 
-/* A PCI device has it's own struct device and so does a virtio device so
- * we create a place for the virtio devices to show up in sysfs.  I think it
- * would make more sense for virtio to not insist on having it's own device. */
-static struct device *virtio_pci_root;
-
 /* Convert a generic virtio device to our structure */
 static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
 {

@@ -629,7 +624,7 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
 	if (vp_dev == NULL)
 		return -ENOMEM;
 
-	vp_dev->vdev.dev.parent = virtio_pci_root;
+	vp_dev->vdev.dev.parent = &pci_dev->dev;
 	vp_dev->vdev.dev.release = virtio_pci_release_dev;
 	vp_dev->vdev.config = &virtio_pci_config_ops;
 	vp_dev->pci_dev = pci_dev;

@@ -717,17 +712,7 @@ static struct pci_driver virtio_pci_driver = {
 
 static int __init virtio_pci_init(void)
 {
-	int err;
-
-	virtio_pci_root = root_device_register("virtio-pci");
-	if (IS_ERR(virtio_pci_root))
-		return PTR_ERR(virtio_pci_root);
-
-	err = pci_register_driver(&virtio_pci_driver);
-	if (err)
-		root_device_unregister(virtio_pci_root);
-
-	return err;
+	return pci_register_driver(&virtio_pci_driver);
 }
 
 module_init(virtio_pci_init);

@@ -735,7 +720,6 @@ module_init(virtio_pci_init);
 static void __exit virtio_pci_exit(void)
 {
 	pci_unregister_driver(&virtio_pci_driver);
-	root_device_unregister(virtio_pci_root);
 }
 
 module_exit(virtio_pci_exit);

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.

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Signed-off-by Milton Miller <[email protected]>

---

This is an alternative to the patch by Michael S. Tsirkin titled "virtio-pci: add softlinks between virtio and pci"
https://patchwork.kernel.org/patch/454581/

It creates simpler code, uses less memory, and should be even easier use by the installer as it won't have to know a virtio symlink to follow (just follow none).

Compile tested only as I don't have kvm setup.

drivers/virtio/virtio_pci.c +2 -18

index ef8d9d5..4fb5b2b 100644
@@ -96,11 +96,6 @@ static struct pci_device_id virtio_pci_id_table[] = {
 
 MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
 
-/* A PCI device has it's own struct device and so does a virtio device so
- * we create a place for the virtio devices to show up in sysfs.  I think it
- * would make more sense for virtio to not insist on having it's own device. */
-static struct device *virtio_pci_root;
-
 /* Convert a generic virtio device to our structure */
 static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
 {

@@ -629,7 +624,7 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
 	if (vp_dev == NULL)
 		return -ENOMEM;
 
-	vp_dev->vdev.dev.parent = virtio_pci_root;
+	vp_dev->vdev.dev.parent = &pci_dev->dev;
 	vp_dev->vdev.dev.release = virtio_pci_release_dev;
 	vp_dev->vdev.config = &virtio_pci_config_ops;
 	vp_dev->pci_dev = pci_dev;

@@ -717,17 +712,7 @@ static struct pci_driver virtio_pci_driver = {
 
 static int __init virtio_pci_init(void)
 {
-	int err;
-
-	virtio_pci_root = root_device_register("virtio-pci");
-	if (IS_ERR(virtio_pci_root))
-		return PTR_ERR(virtio_pci_root);
-
-	err = pci_register_driver(&virtio_pci_driver);
-	if (err)
-		root_device_unregister(virtio_pci_root);
-
-	return err;
+	return pci_register_driver(&virtio_pci_driver);
 }
 
 module_init(virtio_pci_init);

@@ -735,7 +720,6 @@ module_init(virtio_pci_init);
 static void __exit virtio_pci_exit(void)
 {
 	pci_unregister_driver(&virtio_pci_driver);
-	root_device_unregister(virtio_pci_root);
 }
 
 module_exit(virtio_pci_exit);

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.

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Signed-off-by Milton Miller <[email protected]>

---

This is an alternative to the patch by Michael S. Tsirkin titled "virtio-pci: add softlinks between virtio and pci"
https://patchwork.kernel.org/patch/454581/

It creates simpler code, uses less memory, and should be even easier use by the installer as it won't have to know a virtio symlink to follow (just follow none).

Compile tested only as I don't have kvm setup.

drivers/virtio/virtio_pci.c +2 -18

index ef8d9d5..4fb5b2b 100644
@@ -96,11 +96,6 @@ static struct pci_device_id virtio_pci_id_table[] = {
 
 MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
 
-/* A PCI device has it's own struct device and so does a virtio device so
- * we create a place for the virtio devices to show up in sysfs.  I think it
- * would make more sense for virtio to not insist on having it's own device. */
-static struct device *virtio_pci_root;
-
 /* Convert a generic virtio device to our structure */
 static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
 {

@@ -629,7 +624,7 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
 	if (vp_dev == NULL)
 		return -ENOMEM;
 
-	vp_dev->vdev.dev.parent = virtio_pci_root;
+	vp_dev->vdev.dev.parent = &pci_dev->dev;
 	vp_dev->vdev.dev.release = virtio_pci_release_dev;
 	vp_dev->vdev.config = &virtio_pci_config_ops;
 	vp_dev->pci_dev = pci_dev;

@@ -717,17 +712,7 @@ static struct pci_driver virtio_pci_driver = {
 
 static int __init virtio_pci_init(void)
 {
-	int err;
-
-	virtio_pci_root = root_device_register("virtio-pci");
-	if (IS_ERR(virtio_pci_root))
-		return PTR_ERR(virtio_pci_root);
-
-	err = pci_register_driver(&virtio_pci_driver);
-	if (err)
-		root_device_unregister(virtio_pci_root);
-
-	return err;
+	return pci_register_driver(&virtio_pci_driver);
 }
 
 module_init(virtio_pci_init);

@@ -735,7 +720,6 @@ module_init(virtio_pci_init);
 static void __exit virtio_pci_exit(void)
 {
 	pci_unregister_driver(&virtio_pci_driver);
-	root_device_unregister(virtio_pci_root);
 }
 
 module_exit(virtio_pci_exit);

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.

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Signed-off-by Milton Miller <[email protected]>

---

This is an alternative to the patch by Michael S. Tsirkin titled "virtio-pci: add softlinks between virtio and pci"
https://patchwork.kernel.org/patch/454581/

It creates simpler code, uses less memory, and should be even easier use by the installer as it won't have to know a virtio symlink to follow (just follow none).

Compile tested only as I don't have kvm setup.

drivers/virtio/virtio_pci.c +2 -18

index ef8d9d5..4fb5b2b 100644
@@ -96,11 +96,6 @@ static struct pci_device_id virtio_pci_id_table[] = {
 
 MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
 
-/* A PCI device has it's own struct device and so does a virtio device so
- * we create a place for the virtio devices to show up in sysfs.  I think it
- * would make more sense for virtio to not insist on having it's own device. */
-static struct device *virtio_pci_root;
-
 /* Convert a generic virtio device to our structure */
 static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
 {

@@ -629,7 +624,7 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
 	if (vp_dev == NULL)
 		return -ENOMEM;
 
-	vp_dev->vdev.dev.parent = virtio_pci_root;
+	vp_dev->vdev.dev.parent = &pci_dev->dev;
 	vp_dev->vdev.dev.release = virtio_pci_release_dev;
 	vp_dev->vdev.config = &virtio_pci_config_ops;
 	vp_dev->pci_dev = pci_dev;

@@ -717,17 +712,7 @@ static struct pci_driver virtio_pci_driver = {
 
 static int __init virtio_pci_init(void)
 {
-	int err;
-
-	virtio_pci_root = root_device_register("virtio-pci");
-	if (IS_ERR(virtio_pci_root))
-		return PTR_ERR(virtio_pci_root);
-
-	err = pci_register_driver(&virtio_pci_driver);
-	if (err)
-		root_device_unregister(virtio_pci_root);
-
-	return err;
+	return pci_register_driver(&virtio_pci_driver);
 }
 
 module_init(virtio_pci_init);

@@ -735,7 +720,6 @@ module_init(virtio_pci_init);
 static void __exit virtio_pci_exit(void)
 {
 	pci_unregister_driver(&virtio_pci_driver);
-	root_device_unregister(virtio_pci_root);
 }
 
 module_exit(virtio_pci_exit);

Quoted 6 lines · Milton Miller

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.

Quoted 8 lines

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Signed-off-by Milton Miller <[email protected]>

OK, this works fine for me. I played with options to add compat softlinks under devices/virtio-pci but we still don't get exactly the same layout and since I don't think anyone actually uses them, it's probably ok to just to the simple thing.

Tested/Acked-by: Michael S. Tsirkin <[email protected]>

Rusty, since this help fix at least one user, any chance this can be put in 2.6.38? OK to backport to -stable?

Gleb, could you try this out too?

Quoted 64 lines

---

This is an alternative to the patch by Michael S. Tsirkin titled "virtio-pci: add softlinks between virtio and pci"
https://patchwork.kernel.org/patch/454581/

It creates simpler code, uses less memory, and should be even easier use by the installer as it won't have to know a virtio symlink to follow (just follow none).

Compile tested only as I don't have kvm setup.

drivers/virtio/virtio_pci.c +2 -18

index ef8d9d5..4fb5b2b 100644
@@ -96,11 +96,6 @@ static struct pci_device_id virtio_pci_id_table[] = {
 
 MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
 
-/* A PCI device has it's own struct device and so does a virtio device so
- * we create a place for the virtio devices to show up in sysfs.  I think it
- * would make more sense for virtio to not insist on having it's own device. */
-static struct device *virtio_pci_root;
-
 /* Convert a generic virtio device to our structure */
 static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
 {

@@ -629,7 +624,7 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
 	if (vp_dev == NULL)
 		return -ENOMEM;
 
-	vp_dev->vdev.dev.parent = virtio_pci_root;
+	vp_dev->vdev.dev.parent = &pci_dev->dev;
 	vp_dev->vdev.dev.release = virtio_pci_release_dev;
 	vp_dev->vdev.config = &virtio_pci_config_ops;
 	vp_dev->pci_dev = pci_dev;

@@ -717,17 +712,7 @@ static struct pci_driver virtio_pci_driver = {
 
 static int __init virtio_pci_init(void)
 {
-	int err;
-
-	virtio_pci_root = root_device_register("virtio-pci");
-	if (IS_ERR(virtio_pci_root))
-		return PTR_ERR(virtio_pci_root);
-
-	err = pci_register_driver(&virtio_pci_driver);
-	if (err)
-		root_device_unregister(virtio_pci_root);
-
-	return err;
+	return pci_register_driver(&virtio_pci_driver);
 }
 
 module_init(virtio_pci_init);

@@ -735,7 +720,6 @@ module_init(virtio_pci_init);
 static void __exit virtio_pci_exit(void)
 {
 	pci_unregister_driver(&virtio_pci_driver);
-	root_device_unregister(virtio_pci_root);
 }
 

drivers/virtio/virtio_pci.c module_init(virtio_pci_init); · line 725

Quoted 1 line

drivers/virtio/virtio_pci.c +0 -0

 module_exit(virtio_pci_exit);

Quoted 6 lines · Milton Miller

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.

Quoted 8 lines

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Signed-off-by Milton Miller <[email protected]>

OK, this works fine for me. I played with options to add compat softlinks under devices/virtio-pci but we still don't get exactly the same layout and since I don't think anyone actually uses them, it's probably ok to just to the simple thing.

Tested/Acked-by: Michael S. Tsirkin <[email protected]>

Rusty, since this help fix at least one user, any chance this can be put in 2.6.38? OK to backport to -stable?

Gleb, could you try this out too?

Quoted 64 lines

---

This is an alternative to the patch by Michael S. Tsirkin titled "virtio-pci: add softlinks between virtio and pci"
https://patchwork.kernel.org/patch/454581/

It creates simpler code, uses less memory, and should be even easier use by the installer as it won't have to know a virtio symlink to follow (just follow none).

Compile tested only as I don't have kvm setup.

drivers/virtio/virtio_pci.c +2 -18

index ef8d9d5..4fb5b2b 100644
@@ -96,11 +96,6 @@ static struct pci_device_id virtio_pci_id_table[] = {
 
 MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
 
-/* A PCI device has it's own struct device and so does a virtio device so
- * we create a place for the virtio devices to show up in sysfs.  I think it
- * would make more sense for virtio to not insist on having it's own device. */
-static struct device *virtio_pci_root;
-
 /* Convert a generic virtio device to our structure */
 static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
 {

@@ -629,7 +624,7 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
 	if (vp_dev == NULL)
 		return -ENOMEM;
 
-	vp_dev->vdev.dev.parent = virtio_pci_root;
+	vp_dev->vdev.dev.parent = &pci_dev->dev;
 	vp_dev->vdev.dev.release = virtio_pci_release_dev;
 	vp_dev->vdev.config = &virtio_pci_config_ops;
 	vp_dev->pci_dev = pci_dev;

@@ -717,17 +712,7 @@ static struct pci_driver virtio_pci_driver = {
 
 static int __init virtio_pci_init(void)
 {
-	int err;
-
-	virtio_pci_root = root_device_register("virtio-pci");
-	if (IS_ERR(virtio_pci_root))
-		return PTR_ERR(virtio_pci_root);
-
-	err = pci_register_driver(&virtio_pci_driver);
-	if (err)
-		root_device_unregister(virtio_pci_root);
-
-	return err;
+	return pci_register_driver(&virtio_pci_driver);
 }
 
 module_init(virtio_pci_init);

@@ -735,7 +720,6 @@ module_init(virtio_pci_init);
 static void __exit virtio_pci_exit(void)
 {
 	pci_unregister_driver(&virtio_pci_driver);
-	root_device_unregister(virtio_pci_root);
 }
 

drivers/virtio/virtio_pci.c module_init(virtio_pci_init); · line 725

Quoted 1 line

drivers/virtio/virtio_pci.c +0 -0

 module_exit(virtio_pci_exit);

Quoted 21 lines · Michael S. Tsirkin

Quoted 6 lines · Milton Miller

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.

Quoted 8 lines

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Signed-off-by Milton Miller <[email protected]>

OK, this works fine for me. I played with options to add compat softlinks under devices/virtio-pci but we still don't get exactly the same layout and since I don't think anyone actually uses them, it's probably ok to just to the simple thing.

Quoted 7 lines

Tested/Acked-by: Michael S. Tsirkin <[email protected]>

Rusty, since this help fix at least one user, any chance this can be put in 2.6.38? OK to backport to -stable?

Gleb, could you try this out too?

With this patch if I have 3 virtio disks for a VM I get:
/sys/devices/pci0000:00/0000:00:04.0/virtio0/block/vda /sys/devices/pci0000:00/0000:00:05.0/virtio1/block/vdb /sys/devices/pci0000:00/0000:00:06.0/virtio2/block/vdc

Number after virtio has no much sense. It either should be dropped at all or be always zero in case we will support more then one virtio controller per pci card. In that case each virtio controller will have directories virtio0/virtio1/virtio2... under same pci device directory.

Quoted 64 lines

Quoted 64 lines

---

This is an alternative to the patch by Michael S. Tsirkin titled "virtio-pci: add softlinks between virtio and pci"
https://patchwork.kernel.org/patch/454581/

It creates simpler code, uses less memory, and should be even easier use by the installer as it won't have to know a virtio symlink to follow (just follow none).

Compile tested only as I don't have kvm setup.

drivers/virtio/virtio_pci.c +2 -18

index ef8d9d5..4fb5b2b 100644
@@ -96,11 +96,6 @@ static struct pci_device_id virtio_pci_id_table[] = {
 
 MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
 
-/* A PCI device has it's own struct device and so does a virtio device so
- * we create a place for the virtio devices to show up in sysfs.  I think it
- * would make more sense for virtio to not insist on having it's own device. */
-static struct device *virtio_pci_root;
-
 /* Convert a generic virtio device to our structure */
 static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
 {

@@ -629,7 +624,7 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
 	if (vp_dev == NULL)
 		return -ENOMEM;
 
-	vp_dev->vdev.dev.parent = virtio_pci_root;
+	vp_dev->vdev.dev.parent = &pci_dev->dev;
 	vp_dev->vdev.dev.release = virtio_pci_release_dev;
 	vp_dev->vdev.config = &virtio_pci_config_ops;
 	vp_dev->pci_dev = pci_dev;

@@ -717,17 +712,7 @@ static struct pci_driver virtio_pci_driver = {
 
 static int __init virtio_pci_init(void)
 {
-	int err;
-
-	virtio_pci_root = root_device_register("virtio-pci");
-	if (IS_ERR(virtio_pci_root))
-		return PTR_ERR(virtio_pci_root);
-
-	err = pci_register_driver(&virtio_pci_driver);
-	if (err)
-		root_device_unregister(virtio_pci_root);
-
-	return err;
+	return pci_register_driver(&virtio_pci_driver);
 }
 
 module_init(virtio_pci_init);

@@ -735,7 +720,6 @@ module_init(virtio_pci_init);
 static void __exit virtio_pci_exit(void)
 {
 	pci_unregister_driver(&virtio_pci_driver);
-	root_device_unregister(virtio_pci_root);
 }
 

Quoted 1 line

drivers/virtio/virtio_pci.c module_init(virtio_pci_init); · line 725

Quoted 1 line

drivers/virtio/virtio_pci.c +0 -0

 module_exit(virtio_pci_exit);

--
Gleb.

Quoted 21 lines · Michael S. Tsirkin

Quoted 6 lines · Milton Miller

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.

Quoted 8 lines

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Signed-off-by Milton Miller <[email protected]>

OK, this works fine for me. I played with options to add compat softlinks under devices/virtio-pci but we still don't get exactly the same layout and since I don't think anyone actually uses them, it's probably ok to just to the simple thing.

Quoted 7 lines

Tested/Acked-by: Michael S. Tsirkin <[email protected]>

Rusty, since this help fix at least one user, any chance this can be put in 2.6.38? OK to backport to -stable?

Gleb, could you try this out too?

With this patch if I have 3 virtio disks for a VM I get:
/sys/devices/pci0000:00/0000:00:04.0/virtio0/block/vda /sys/devices/pci0000:00/0000:00:05.0/virtio1/block/vdb /sys/devices/pci0000:00/0000:00:06.0/virtio2/block/vdc

Number after virtio has no much sense. It either should be dropped at all or be always zero in case we will support more then one virtio controller per pci card. In that case each virtio controller will have directories virtio0/virtio1/virtio2... under same pci device directory.

Quoted 64 lines

Quoted 64 lines

---

This is an alternative to the patch by Michael S. Tsirkin titled "virtio-pci: add softlinks between virtio and pci"
https://patchwork.kernel.org/patch/454581/

It creates simpler code, uses less memory, and should be even easier use by the installer as it won't have to know a virtio symlink to follow (just follow none).

Compile tested only as I don't have kvm setup.

drivers/virtio/virtio_pci.c +2 -18

index ef8d9d5..4fb5b2b 100644
@@ -96,11 +96,6 @@ static struct pci_device_id virtio_pci_id_table[] = {
 
 MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
 
-/* A PCI device has it's own struct device and so does a virtio device so
- * we create a place for the virtio devices to show up in sysfs.  I think it
- * would make more sense for virtio to not insist on having it's own device. */
-static struct device *virtio_pci_root;
-
 /* Convert a generic virtio device to our structure */
 static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
 {

@@ -629,7 +624,7 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
 	if (vp_dev == NULL)
 		return -ENOMEM;
 
-	vp_dev->vdev.dev.parent = virtio_pci_root;
+	vp_dev->vdev.dev.parent = &pci_dev->dev;
 	vp_dev->vdev.dev.release = virtio_pci_release_dev;
 	vp_dev->vdev.config = &virtio_pci_config_ops;
 	vp_dev->pci_dev = pci_dev;

@@ -717,17 +712,7 @@ static struct pci_driver virtio_pci_driver = {
 
 static int __init virtio_pci_init(void)
 {
-	int err;
-
-	virtio_pci_root = root_device_register("virtio-pci");
-	if (IS_ERR(virtio_pci_root))
-		return PTR_ERR(virtio_pci_root);
-
-	err = pci_register_driver(&virtio_pci_driver);
-	if (err)
-		root_device_unregister(virtio_pci_root);
-
-	return err;
+	return pci_register_driver(&virtio_pci_driver);
 }
 
 module_init(virtio_pci_init);

@@ -735,7 +720,6 @@ module_init(virtio_pci_init);
 static void __exit virtio_pci_exit(void)
 {
 	pci_unregister_driver(&virtio_pci_driver);
-	root_device_unregister(virtio_pci_root);
 }
 

Quoted 1 line

drivers/virtio/virtio_pci.c module_init(virtio_pci_init); · line 725

Quoted 1 line

drivers/virtio/virtio_pci.c +0 -0

 module_exit(virtio_pci_exit);

--
Gleb.

Quoted 34 lines · Gleb Natapov

Quoted 21 lines · Michael S. Tsirkin

Quoted 6 lines · Milton Miller

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.

Quoted 8 lines

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Signed-off-by Milton Miller <[email protected]>

OK, this works fine for me. I played with options to add compat softlinks under devices/virtio-pci but we still don't get exactly the same layout and since I don't think anyone actually uses them, it's probably ok to just to the simple thing.

Quoted 7 lines

Tested/Acked-by: Michael S. Tsirkin <[email protected]>

Rusty, since this help fix at least one user, any chance this can be put in 2.6.38? OK to backport to -stable?

Gleb, could you try this out too?

With this patch if I have 3 virtio disks for a VM I get:
/sys/devices/pci0000:00/0000:00:04.0/virtio0/block/vda /sys/devices/pci0000:00/0000:00:05.0/virtio1/block/vdb /sys/devices/pci0000:00/0000:00:06.0/virtio2/block/vdc

Quoted 4 lines

Number after virtio has no much sense. It either should be dropped at all or be always zero in case we will support more then one virtio controller per pci card. In that case each virtio controller will have directories virtio0/virtio1/virtio2... under same pci device directory.

Yes. But this is the bus name. It must be unique - all devices also appear under /sys/bus/virtio/devices.

Quoted 64 lines

Quoted 64 lines

Quoted 64 lines

---

This is an alternative to the patch by Michael S. Tsirkin titled "virtio-pci: add softlinks between virtio and pci"
https://patchwork.kernel.org/patch/454581/

It creates simpler code, uses less memory, and should be even easier use by the installer as it won't have to know a virtio symlink to follow (just follow none).

Compile tested only as I don't have kvm setup.

drivers/virtio/virtio_pci.c +2 -18

index ef8d9d5..4fb5b2b 100644
@@ -96,11 +96,6 @@ static struct pci_device_id virtio_pci_id_table[] = {
 
 MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
 
-/* A PCI device has it's own struct device and so does a virtio device so
- * we create a place for the virtio devices to show up in sysfs.  I think it
- * would make more sense for virtio to not insist on having it's own device. */
-static struct device *virtio_pci_root;
-
 /* Convert a generic virtio device to our structure */
 static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
 {

@@ -629,7 +624,7 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
 	if (vp_dev == NULL)
 		return -ENOMEM;
 
-	vp_dev->vdev.dev.parent = virtio_pci_root;
+	vp_dev->vdev.dev.parent = &pci_dev->dev;
 	vp_dev->vdev.dev.release = virtio_pci_release_dev;
 	vp_dev->vdev.config = &virtio_pci_config_ops;
 	vp_dev->pci_dev = pci_dev;

@@ -717,17 +712,7 @@ static struct pci_driver virtio_pci_driver = {
 
 static int __init virtio_pci_init(void)
 {
-	int err;
-
-	virtio_pci_root = root_device_register("virtio-pci");
-	if (IS_ERR(virtio_pci_root))
-		return PTR_ERR(virtio_pci_root);
-
-	err = pci_register_driver(&virtio_pci_driver);
-	if (err)
-		root_device_unregister(virtio_pci_root);
-
-	return err;
+	return pci_register_driver(&virtio_pci_driver);
 }
 
 module_init(virtio_pci_init);

@@ -735,7 +720,6 @@ module_init(virtio_pci_init);
 static void __exit virtio_pci_exit(void)
 {
 	pci_unregister_driver(&virtio_pci_driver);
-	root_device_unregister(virtio_pci_root);
 }
 

Quoted 4 lines

Quoted 1 line

drivers/virtio/virtio_pci.c module_init(virtio_pci_init); · line 725

Quoted 1 line

drivers/virtio/virtio_pci.c +0 -0

 module_exit(virtio_pci_exit);

--
Gleb.

Quoted 34 lines · Gleb Natapov

Quoted 21 lines · Michael S. Tsirkin

Quoted 6 lines · Milton Miller

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.

Quoted 8 lines

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Signed-off-by Milton Miller <[email protected]>

OK, this works fine for me. I played with options to add compat softlinks under devices/virtio-pci but we still don't get exactly the same layout and since I don't think anyone actually uses them, it's probably ok to just to the simple thing.

Quoted 7 lines

Tested/Acked-by: Michael S. Tsirkin <[email protected]>

Rusty, since this help fix at least one user, any chance this can be put in 2.6.38? OK to backport to -stable?

Gleb, could you try this out too?

With this patch if I have 3 virtio disks for a VM I get:
/sys/devices/pci0000:00/0000:00:04.0/virtio0/block/vda /sys/devices/pci0000:00/0000:00:05.0/virtio1/block/vdb /sys/devices/pci0000:00/0000:00:06.0/virtio2/block/vdc

Quoted 4 lines

Number after virtio has no much sense. It either should be dropped at all or be always zero in case we will support more then one virtio controller per pci card. In that case each virtio controller will have directories virtio0/virtio1/virtio2... under same pci device directory.

Yes. But this is the bus name. It must be unique - all devices also appear under /sys/bus/virtio/devices.

Quoted 64 lines

Quoted 64 lines

Quoted 64 lines

---

This is an alternative to the patch by Michael S. Tsirkin titled "virtio-pci: add softlinks between virtio and pci"
https://patchwork.kernel.org/patch/454581/

It creates simpler code, uses less memory, and should be even easier use by the installer as it won't have to know a virtio symlink to follow (just follow none).

Compile tested only as I don't have kvm setup.

drivers/virtio/virtio_pci.c +2 -18

index ef8d9d5..4fb5b2b 100644
@@ -96,11 +96,6 @@ static struct pci_device_id virtio_pci_id_table[] = {
 
 MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
 
-/* A PCI device has it's own struct device and so does a virtio device so
- * we create a place for the virtio devices to show up in sysfs.  I think it
- * would make more sense for virtio to not insist on having it's own device. */
-static struct device *virtio_pci_root;
-
 /* Convert a generic virtio device to our structure */
 static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
 {

@@ -629,7 +624,7 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
 	if (vp_dev == NULL)
 		return -ENOMEM;
 
-	vp_dev->vdev.dev.parent = virtio_pci_root;
+	vp_dev->vdev.dev.parent = &pci_dev->dev;
 	vp_dev->vdev.dev.release = virtio_pci_release_dev;
 	vp_dev->vdev.config = &virtio_pci_config_ops;
 	vp_dev->pci_dev = pci_dev;

@@ -717,17 +712,7 @@ static struct pci_driver virtio_pci_driver = {
 
 static int __init virtio_pci_init(void)
 {
-	int err;
-
-	virtio_pci_root = root_device_register("virtio-pci");
-	if (IS_ERR(virtio_pci_root))
-		return PTR_ERR(virtio_pci_root);
-
-	err = pci_register_driver(&virtio_pci_driver);
-	if (err)
-		root_device_unregister(virtio_pci_root);
-
-	return err;
+	return pci_register_driver(&virtio_pci_driver);
 }
 
 module_init(virtio_pci_init);

@@ -735,7 +720,6 @@ module_init(virtio_pci_init);
 static void __exit virtio_pci_exit(void)
 {
 	pci_unregister_driver(&virtio_pci_driver);
-	root_device_unregister(virtio_pci_root);
 }
 

Quoted 4 lines

Quoted 1 line

drivers/virtio/virtio_pci.c module_init(virtio_pci_init); · line 725

Quoted 1 line

drivers/virtio/virtio_pci.c +0 -0

 module_exit(virtio_pci_exit);

--
Gleb.

Quoted 40 lines · Michael S. Tsirkin

Quoted 38 lines · Gleb Natapov

Quoted 21 lines · Michael S. Tsirkin

Quoted 6 lines · Milton Miller

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.

Quoted 8 lines

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Signed-off-by Milton Miller <[email protected]>

OK, this works fine for me. I played with options to add compat softlinks under devices/virtio-pci but we still don't get exactly the same layout and since I don't think anyone actually uses them, it's probably ok to just to the simple thing.

Quoted 7 lines

Tested/Acked-by: Michael S. Tsirkin <[email protected]>

Rusty, since this help fix at least one user, any chance this can be put in 2.6.38? OK to backport to -stable?

Gleb, could you try this out too?

With this patch if I have 3 virtio disks for a VM I get:
/sys/devices/pci0000:00/0000:00:04.0/virtio0/block/vda /sys/devices/pci0000:00/0000:00:05.0/virtio1/block/vdb /sys/devices/pci0000:00/0000:00:06.0/virtio2/block/vdc

Number after virtio has no much sense. It either should be dropped at all or be always zero in case we will support more then one virtio controller per pci card. In that case each virtio controller will have directories virtio0/virtio1/virtio2... under same pci device directory.

Quoted 3 lines

Yes. But this is the bus name. It must be unique - all devices also appear under /sys/bus/virtio/devices.

It is very strange king of bus that is spread over several PCI devices :) It doesn't make much sense IMHO, but I can leave with it.

Quoted 41 lines · Gleb Natapov

Quoted 40 lines · Michael S. Tsirkin

Quoted 38 lines · Gleb Natapov

Quoted 21 lines · Michael S. Tsirkin

Quoted 6 lines · Milton Miller

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.

Quoted 8 lines

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Signed-off-by Milton Miller <[email protected]>

OK, this works fine for me. I played with options to add compat softlinks under devices/virtio-pci but we still don't get exactly the same layout and since I don't think anyone actually uses them, it's probably ok to just to the simple thing.

Quoted 7 lines

Tested/Acked-by: Michael S. Tsirkin <[email protected]>

Rusty, since this help fix at least one user, any chance this can be put in 2.6.38? OK to backport to -stable?

Gleb, could you try this out too?

With this patch if I have 3 virtio disks for a VM I get:
/sys/devices/pci0000:00/0000:00:04.0/virtio0/block/vda /sys/devices/pci0000:00/0000:00:05.0/virtio1/block/vdb /sys/devices/pci0000:00/0000:00:06.0/virtio2/block/vdc

Number after virtio has no much sense. It either should be dropped at all or be always zero in case we will support more then one virtio controller per pci card. In that case each virtio controller will have directories virtio0/virtio1/virtio2... under same pci device directory.

Quoted 6 lines

Quoted 3 lines

Yes. But this is the bus name. It must be unique - all devices also appear under /sys/bus/virtio/devices.

It is very strange king of bus that is spread over several PCI devices :) It doesn't make much sense IMHO, but I can leave with it.

I can't "leave" with it, but I can "live" with it.

--
Gleb.

Quoted 41 lines · Gleb Natapov

Quoted 40 lines · Michael S. Tsirkin

Quoted 38 lines · Gleb Natapov

Quoted 21 lines · Michael S. Tsirkin

Quoted 6 lines · Milton Miller

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.

Quoted 8 lines

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Signed-off-by Milton Miller <[email protected]>

OK, this works fine for me. I played with options to add compat softlinks under devices/virtio-pci but we still don't get exactly the same layout and since I don't think anyone actually uses them, it's probably ok to just to the simple thing.

Quoted 7 lines

Tested/Acked-by: Michael S. Tsirkin <[email protected]>

Rusty, since this help fix at least one user, any chance this can be put in 2.6.38? OK to backport to -stable?

Gleb, could you try this out too?

With this patch if I have 3 virtio disks for a VM I get:
/sys/devices/pci0000:00/0000:00:04.0/virtio0/block/vda /sys/devices/pci0000:00/0000:00:05.0/virtio1/block/vdb /sys/devices/pci0000:00/0000:00:06.0/virtio2/block/vdc

Number after virtio has no much sense. It either should be dropped at all or be always zero in case we will support more then one virtio controller per pci card. In that case each virtio controller will have directories virtio0/virtio1/virtio2... under same pci device directory.

Quoted 6 lines

Quoted 3 lines

Yes. But this is the bus name. It must be unique - all devices also appear under /sys/bus/virtio/devices.

It is very strange king of bus that is spread over several PCI devices :) It doesn't make much sense IMHO, but I can leave with it.

I can't "leave" with it, but I can "live" with it.

--
Gleb.

Quoted 48 lines · Gleb Natapov

Quoted 47 lines · Gleb Natapov

Quoted 35 lines · Michael S. Tsirkin

Quoted 34 lines · Gleb Natapov

Quoted 21 lines · Michael S. Tsirkin

Quoted 6 lines · Milton Miller

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.

Quoted 8 lines

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Signed-off-by Milton Miller <[email protected]>

OK, this works fine for me. I played with options to add compat softlinks under devices/virtio-pci but we still don't get exactly the same layout and since I don't think anyone actually uses them, it's probably ok to just to the simple thing.

Quoted 7 lines

Tested/Acked-by: Michael S. Tsirkin <[email protected]>

Rusty, since this help fix at least one user, any chance this can be put in 2.6.38? OK to backport to -stable?

Gleb, could you try this out too?

With this patch if I have 3 virtio disks for a VM I get:
/sys/devices/pci0000:00/0000:00:04.0/virtio0/block/vda /sys/devices/pci0000:00/0000:00:05.0/virtio1/block/vdb /sys/devices/pci0000:00/0000:00:06.0/virtio2/block/vdc

Quoted 8 lines

Quoted 4 lines

Number after virtio has no much sense. It either should be dropped at all or be always zero in case we will support more then one virtio controller per pci card. In that case each virtio controller will have directories virtio0/virtio1/virtio2... under same pci device directory.

Yes. But this is the bus name. It must be unique - all devices also appear under /sys/bus/virtio/devices.

It is very strange king of bus that is spread over several PCI devices :) It doesn't make much sense IMHO, but I can leave with it.

Quoted 1 line

I can't "leave" with it, but I can "live" with it.

The virtio bus is an attempt to make as many applications as possible work transparently on any virtio system,
be it lguest, s390 or pci. Arbitrary IDs is just a hint to the applications 'don't rely on the name at all'.

Quoted 48 lines · Gleb Natapov

Quoted 47 lines · Gleb Natapov

Quoted 35 lines · Michael S. Tsirkin

Quoted 34 lines · Gleb Natapov

Quoted 21 lines · Michael S. Tsirkin

Quoted 6 lines · Milton Miller

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.

Quoted 8 lines

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Signed-off-by Milton Miller <[email protected]>

OK, this works fine for me. I played with options to add compat softlinks under devices/virtio-pci but we still don't get exactly the same layout and since I don't think anyone actually uses them, it's probably ok to just to the simple thing.

Quoted 7 lines

Tested/Acked-by: Michael S. Tsirkin <[email protected]>

Rusty, since this help fix at least one user, any chance this can be put in 2.6.38? OK to backport to -stable?

Gleb, could you try this out too?

With this patch if I have 3 virtio disks for a VM I get:
/sys/devices/pci0000:00/0000:00:04.0/virtio0/block/vda /sys/devices/pci0000:00/0000:00:05.0/virtio1/block/vdb /sys/devices/pci0000:00/0000:00:06.0/virtio2/block/vdc

Quoted 8 lines

Quoted 4 lines

Number after virtio has no much sense. It either should be dropped at all or be always zero in case we will support more then one virtio controller per pci card. In that case each virtio controller will have directories virtio0/virtio1/virtio2... under same pci device directory.

Yes. But this is the bus name. It must be unique - all devices also appear under /sys/bus/virtio/devices.

It is very strange king of bus that is spread over several PCI devices :) It doesn't make much sense IMHO, but I can leave with it.

Quoted 1 line

I can't "leave" with it, but I can "live" with it.

The virtio bus is an attempt to make as many applications as possible work transparently on any virtio system,
be it lguest, s390 or pci. Arbitrary IDs is just a hint to the applications 'don't rely on the name at all'.

Quoted 49 lines · Michael S. Tsirkin

Quoted 48 lines · Gleb Natapov

Quoted 47 lines · Gleb Natapov

Quoted 35 lines · Michael S. Tsirkin

Quoted 34 lines · Gleb Natapov

Quoted 21 lines · Michael S. Tsirkin

Quoted 6 lines · Milton Miller

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.

Quoted 8 lines

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Signed-off-by Milton Miller <[email protected]>

OK, this works fine for me. I played with options to add compat softlinks under devices/virtio-pci but we still don't get exactly the same layout and since I don't think anyone actually uses them, it's probably ok to just to the simple thing.

Quoted 7 lines

Tested/Acked-by: Michael S. Tsirkin <[email protected]>

Rusty, since this help fix at least one user, any chance this can be put in 2.6.38? OK to backport to -stable?

Gleb, could you try this out too?

With this patch if I have 3 virtio disks for a VM I get:
/sys/devices/pci0000:00/0000:00:04.0/virtio0/block/vda /sys/devices/pci0000:00/0000:00:05.0/virtio1/block/vdb /sys/devices/pci0000:00/0000:00:06.0/virtio2/block/vdc

Quoted 8 lines

Quoted 4 lines

Number after virtio has no much sense. It either should be dropped at all or be always zero in case we will support more then one virtio controller per pci card. In that case each virtio controller will have directories virtio0/virtio1/virtio2... under same pci device directory.

Yes. But this is the bus name. It must be unique - all devices also appear under /sys/bus/virtio/devices.

It is very strange king of bus that is spread over several PCI devices :) It doesn't make much sense IMHO, but I can leave with it.

Quoted 7 lines

Quoted 1 line

I can't "leave" with it, but I can "live" with it.

The virtio bus is an attempt to make as many applications as possible work transparently on any virtio system,
be it lguest, s390 or pci. Arbitrary IDs is just a hint to the applications 'don't rely on the name at all'.

Shouldn't sysfs directory reflect real device topology?

There are other buses AFAIK that connected differently on different HW. How virtio is special?
What applications this allow to work transparently?

--
Gleb.

Quoted 49 lines · Michael S. Tsirkin

Quoted 48 lines · Gleb Natapov

Quoted 47 lines · Gleb Natapov

Quoted 35 lines · Michael S. Tsirkin

Quoted 34 lines · Gleb Natapov

Quoted 21 lines · Michael S. Tsirkin

Quoted 6 lines · Milton Miller

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.

Quoted 8 lines

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Signed-off-by Milton Miller <[email protected]>

OK, this works fine for me. I played with options to add compat softlinks under devices/virtio-pci but we still don't get exactly the same layout and since I don't think anyone actually uses them, it's probably ok to just to the simple thing.

Quoted 7 lines

Tested/Acked-by: Michael S. Tsirkin <[email protected]>

Rusty, since this help fix at least one user, any chance this can be put in 2.6.38? OK to backport to -stable?

Gleb, could you try this out too?

With this patch if I have 3 virtio disks for a VM I get:
/sys/devices/pci0000:00/0000:00:04.0/virtio0/block/vda /sys/devices/pci0000:00/0000:00:05.0/virtio1/block/vdb /sys/devices/pci0000:00/0000:00:06.0/virtio2/block/vdc

Quoted 8 lines

Quoted 4 lines

Number after virtio has no much sense. It either should be dropped at all or be always zero in case we will support more then one virtio controller per pci card. In that case each virtio controller will have directories virtio0/virtio1/virtio2... under same pci device directory.

Yes. But this is the bus name. It must be unique - all devices also appear under /sys/bus/virtio/devices.

It is very strange king of bus that is spread over several PCI devices :) It doesn't make much sense IMHO, but I can leave with it.

Quoted 7 lines

Quoted 1 line

I can't "leave" with it, but I can "live" with it.

The virtio bus is an attempt to make as many applications as possible work transparently on any virtio system,
be it lguest, s390 or pci. Arbitrary IDs is just a hint to the applications 'don't rely on the name at all'.

Shouldn't sysfs directory reflect real device topology?

There are other buses AFAIK that connected differently on different HW. How virtio is special?
What applications this allow to work transparently?

--
Gleb.

Quoted 57 lines · Gleb Natapov

Quoted 56 lines · Michael S. Tsirkin

Quoted 42 lines · Gleb Natapov

Quoted 41 lines · Gleb Natapov

Quoted 40 lines · Michael S. Tsirkin

Quoted 38 lines · Gleb Natapov

Quoted 21 lines · Michael S. Tsirkin

Quoted 6 lines · Milton Miller

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.

Quoted 8 lines

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Signed-off-by Milton Miller <[email protected]>

OK, this works fine for me. I played with options to add compat softlinks under devices/virtio-pci but we still don't get exactly the same layout and since I don't think anyone actually uses them, it's probably ok to just to the simple thing.

Quoted 7 lines

Tested/Acked-by: Michael S. Tsirkin <[email protected]>

Rusty, since this help fix at least one user, any chance this can be put in 2.6.38? OK to backport to -stable?

Gleb, could you try this out too?

With this patch if I have 3 virtio disks for a VM I get:
/sys/devices/pci0000:00/0000:00:04.0/virtio0/block/vda /sys/devices/pci0000:00/0000:00:05.0/virtio1/block/vdb /sys/devices/pci0000:00/0000:00:06.0/virtio2/block/vdc

Number after virtio has no much sense. It either should be dropped at all or be always zero in case we will support more then one virtio controller per pci card. In that case each virtio controller will have directories virtio0/virtio1/virtio2... under same pci device directory.

Quoted 7 lines

Quoted 6 lines

Quoted 3 lines

Yes. But this is the bus name. It must be unique - all devices also appear under /sys/bus/virtio/devices.

It is very strange king of bus that is spread over several PCI devices :) It doesn't make much sense IMHO, but I can leave with it.

I can't "leave" with it, but I can "live" with it.

The virtio bus is an attempt to make as many applications as possible work transparently on any virtio system,
be it lguest, s390 or pci. Arbitrary IDs is just a hint to the applications 'don't rely on the name at all'.

Quoted 1 line

Shouldn't sysfs directory reflect real device topology?

What exactly is proposed here? Are you acking or nacking this patch? Prefer the older version? Can send your own?

Quoted 3 lines

There are other buses AFAIK that connected differently on different HW. How virtio is special?
What applications this allow to work transparently?

I don't know which applications use the current sysfs topology but the assumption that there are none and we can break it arbitrarily seems a bit drastic.

Quoted 2 lines

--
Gleb.

Quoted 57 lines · Gleb Natapov

Quoted 56 lines · Michael S. Tsirkin

Quoted 42 lines · Gleb Natapov

Quoted 41 lines · Gleb Natapov

Quoted 40 lines · Michael S. Tsirkin

Quoted 38 lines · Gleb Natapov

Quoted 21 lines · Michael S. Tsirkin

Quoted 6 lines · Milton Miller

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.

Quoted 8 lines

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Signed-off-by Milton Miller <[email protected]>

OK, this works fine for me. I played with options to add compat softlinks under devices/virtio-pci but we still don't get exactly the same layout and since I don't think anyone actually uses them, it's probably ok to just to the simple thing.

Quoted 7 lines

Tested/Acked-by: Michael S. Tsirkin <[email protected]>

Rusty, since this help fix at least one user, any chance this can be put in 2.6.38? OK to backport to -stable?

Gleb, could you try this out too?

With this patch if I have 3 virtio disks for a VM I get:
/sys/devices/pci0000:00/0000:00:04.0/virtio0/block/vda /sys/devices/pci0000:00/0000:00:05.0/virtio1/block/vdb /sys/devices/pci0000:00/0000:00:06.0/virtio2/block/vdc

Number after virtio has no much sense. It either should be dropped at all or be always zero in case we will support more then one virtio controller per pci card. In that case each virtio controller will have directories virtio0/virtio1/virtio2... under same pci device directory.

Quoted 7 lines

Quoted 6 lines

Quoted 3 lines

Yes. But this is the bus name. It must be unique - all devices also appear under /sys/bus/virtio/devices.

It is very strange king of bus that is spread over several PCI devices :) It doesn't make much sense IMHO, but I can leave with it.

I can't "leave" with it, but I can "live" with it.

The virtio bus is an attempt to make as many applications as possible work transparently on any virtio system,
be it lguest, s390 or pci. Arbitrary IDs is just a hint to the applications 'don't rely on the name at all'.

Quoted 1 line

Shouldn't sysfs directory reflect real device topology?

What exactly is proposed here? Are you acking or nacking this patch? Prefer the older version? Can send your own?

Quoted 3 lines

There are other buses AFAIK that connected differently on different HW. How virtio is special?
What applications this allow to work transparently?

I don't know which applications use the current sysfs topology but the assumption that there are none and we can break it arbitrarily seems a bit drastic.

Quoted 2 lines

--
Gleb.

Quoted 58 lines · Michael S. Tsirkin

Quoted 57 lines · Gleb Natapov

Quoted 56 lines · Michael S. Tsirkin

Quoted 42 lines · Gleb Natapov

Quoted 41 lines · Gleb Natapov

Quoted 40 lines · Michael S. Tsirkin

Quoted 38 lines · Gleb Natapov

Quoted 21 lines · Michael S. Tsirkin

Quoted 6 lines · Milton Miller

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.

Quoted 8 lines

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Signed-off-by Milton Miller <[email protected]>

OK, this works fine for me. I played with options to add compat softlinks under devices/virtio-pci but we still don't get exactly the same layout and since I don't think anyone actually uses them, it's probably ok to just to the simple thing.

Quoted 7 lines

Tested/Acked-by: Michael S. Tsirkin <[email protected]>

Rusty, since this help fix at least one user, any chance this can be put in 2.6.38? OK to backport to -stable?

Gleb, could you try this out too?

With this patch if I have 3 virtio disks for a VM I get:
/sys/devices/pci0000:00/0000:00:04.0/virtio0/block/vda /sys/devices/pci0000:00/0000:00:05.0/virtio1/block/vdb /sys/devices/pci0000:00/0000:00:06.0/virtio2/block/vdc

Number after virtio has no much sense. It either should be dropped at all or be always zero in case we will support more then one virtio controller per pci card. In that case each virtio controller will have directories virtio0/virtio1/virtio2... under same pci device directory.

Quoted 7 lines

Quoted 6 lines

Quoted 3 lines

Yes. But this is the bus name. It must be unique - all devices also appear under /sys/bus/virtio/devices.

It is very strange king of bus that is spread over several PCI devices :) It doesn't make much sense IMHO, but I can leave with it.

I can't "leave" with it, but I can "live" with it.

The virtio bus is an attempt to make as many applications as possible work transparently on any virtio system,
be it lguest, s390 or pci. Arbitrary IDs is just a hint to the applications 'don't rely on the name at all'.

Quoted 5 lines

Quoted 1 line

Shouldn't sysfs directory reflect real device topology?

What exactly is proposed here? Are you acking or nacking this patch? Prefer the older version? Can send your own?

What exactly is proposed is to get rid of random number in virtio directory name. If this is complicated, as I said above, I can live with it as is.

Quoted 10 lines

Quoted 3 lines

There are other buses AFAIK that connected differently on different HW. How virtio is special?
What applications this allow to work transparently?

I don't know which applications use the current sysfs topology but the assumption that there are none and we can break it arbitrarily seems a bit drastic.

Quoted 2 lines

--
Gleb.

--
Gleb.

Quoted 58 lines · Michael S. Tsirkin

Quoted 57 lines · Gleb Natapov

Quoted 56 lines · Michael S. Tsirkin

Quoted 42 lines · Gleb Natapov

Quoted 41 lines · Gleb Natapov

Quoted 40 lines · Michael S. Tsirkin

Quoted 38 lines · Gleb Natapov

Quoted 21 lines · Michael S. Tsirkin

Quoted 6 lines · Milton Miller

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.

Quoted 8 lines

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Signed-off-by Milton Miller <[email protected]>

OK, this works fine for me. I played with options to add compat softlinks under devices/virtio-pci but we still don't get exactly the same layout and since I don't think anyone actually uses them, it's probably ok to just to the simple thing.

Quoted 7 lines

Tested/Acked-by: Michael S. Tsirkin <[email protected]>

Rusty, since this help fix at least one user, any chance this can be put in 2.6.38? OK to backport to -stable?

Gleb, could you try this out too?

With this patch if I have 3 virtio disks for a VM I get:
/sys/devices/pci0000:00/0000:00:04.0/virtio0/block/vda /sys/devices/pci0000:00/0000:00:05.0/virtio1/block/vdb /sys/devices/pci0000:00/0000:00:06.0/virtio2/block/vdc

Number after virtio has no much sense. It either should be dropped at all or be always zero in case we will support more then one virtio controller per pci card. In that case each virtio controller will have directories virtio0/virtio1/virtio2... under same pci device directory.

Quoted 7 lines

Quoted 6 lines

Quoted 3 lines

Yes. But this is the bus name. It must be unique - all devices also appear under /sys/bus/virtio/devices.

It is very strange king of bus that is spread over several PCI devices :) It doesn't make much sense IMHO, but I can leave with it.

I can't "leave" with it, but I can "live" with it.

The virtio bus is an attempt to make as many applications as possible work transparently on any virtio system,
be it lguest, s390 or pci. Arbitrary IDs is just a hint to the applications 'don't rely on the name at all'.

Quoted 5 lines

Quoted 1 line

Shouldn't sysfs directory reflect real device topology?

What exactly is proposed here? Are you acking or nacking this patch? Prefer the older version? Can send your own?

What exactly is proposed is to get rid of random number in virtio directory name. If this is complicated, as I said above, I can live with it as is.

Quoted 10 lines

Quoted 3 lines

There are other buses AFAIK that connected differently on different HW. How virtio is special?
What applications this allow to work transparently?

I don't know which applications use the current sysfs topology but the assumption that there are none and we can break it arbitrarily seems a bit drastic.

Quoted 2 lines

--
Gleb.

--
Gleb.

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.

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Signed-off-by Milton Miller <[email protected]>

---

This is an alternative to the patch by Michael S. Tsirkin titled "virtio-pci: add softlinks between virtio and pci"
https://patchwork.kernel.org/patch/454581/

It creates simpler code, uses less memory, and should be even easier use by the installer as it won't have to know a virtio symlink to follow (just follow none).

Compile tested only as I don't have kvm setup.

drivers/virtio/virtio_pci.c +2 -17

index ef8d9d5..4fb5b2b 100644
@@ -96,11 +96,6 @@ static struct pci_device_id virtio_pci_id_table[] = {
 
 MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
 
-/* A PCI device has it's own struct device and so does a virtio device so
- * we create a place for the virtio devices to show up in sysfs.  I think it
- * would make more sense for virtio to not insist on having it's own device. */
-static struct device *virtio_pci_root;
-
 /* Convert a generic virtio device to our structure */
 static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
 {

@@ -629,7 +624,7 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
 	if (vp_dev == NULL)
 		return -ENOMEM;
 
-	vp_dev->vdev.dev.parent = virtio_pci_root;
+	vp_dev->vdev.dev.parent = &pci_dev->dev;
 	vp_dev->vdev.dev.release = virtio_pci_release_dev;
 	vp_dev->vdev.config = &virtio_pci_config_ops;
 	vp_dev->pci_dev = pci_dev;

@@ -717,17 +712,7 @@ static struct pci_driver virtio_pci_driver = {
 
 static int __init virtio_pci_init(void)
 {
-	int err;
-
-	virtio_pci_root = root_device_register("virtio-pci");
-	if (IS_ERR(virtio_pci_root))
-		return PTR_ERR(virtio_pci_root);
-
-	err = pci_register_driver(&virtio_pci_driver);
-	if (err)
-		root_device_unregister(virtio_pci_root);
-
-	return err;
+	return pci_register_driver(&virtio_pci_driver);
 }
 
 module_init(virtio_pci_init);

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.

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Signed-off-by Milton Miller <[email protected]>

---

This is an alternative to the patch by Michael S. Tsirkin titled "virtio-pci: add softlinks between virtio and pci"
https://patchwork.kernel.org/patch/454581/

It creates simpler code, uses less memory, and should be even easier use by the installer as it won't have to know a virtio symlink to follow (just follow none).

Compile tested only as I don't have kvm setup.

drivers/virtio/virtio_pci.c +2 -17

index ef8d9d5..4fb5b2b 100644
@@ -96,11 +96,6 @@ static struct pci_device_id virtio_pci_id_table[] = {
 
 MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
 
-/* A PCI device has it's own struct device and so does a virtio device so
- * we create a place for the virtio devices to show up in sysfs.  I think it
- * would make more sense for virtio to not insist on having it's own device. */
-static struct device *virtio_pci_root;
-
 /* Convert a generic virtio device to our structure */
 static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
 {

@@ -629,7 +624,7 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
 	if (vp_dev == NULL)
 		return -ENOMEM;
 
-	vp_dev->vdev.dev.parent = virtio_pci_root;
+	vp_dev->vdev.dev.parent = &pci_dev->dev;
 	vp_dev->vdev.dev.release = virtio_pci_release_dev;
 	vp_dev->vdev.config = &virtio_pci_config_ops;
 	vp_dev->pci_dev = pci_dev;

@@ -717,17 +712,7 @@ static struct pci_driver virtio_pci_driver = {
 
 static int __init virtio_pci_init(void)
 {
-	int err;
-
-	virtio_pci_root = root_device_register("virtio-pci");
-	if (IS_ERR(virtio_pci_root))
-		return PTR_ERR(virtio_pci_root);
-
-	err = pci_register_driver(&virtio_pci_driver);
-	if (err)
-		root_device_unregister(virtio_pci_root);
-
-	return err;
+	return pci_register_driver(&virtio_pci_driver);
 }
 
 module_init(virtio_pci_init);

Quoted 13 lines · Milton Miller

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.

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Quoted 1 line

Signed-off-by Milton Miller <[email protected]>

Acked-by Gleb Natapov <[email protected]>

Quoted 58 lines

---

This is an alternative to the patch by Michael S. Tsirkin titled "virtio-pci: add softlinks between virtio and pci"
https://patchwork.kernel.org/patch/454581/

It creates simpler code, uses less memory, and should be even easier use by the installer as it won't have to know a virtio symlink to follow (just follow none).

Compile tested only as I don't have kvm setup.

drivers/virtio/virtio_pci.c +2 -17

index ef8d9d5..4fb5b2b 100644
@@ -96,11 +96,6 @@ static struct pci_device_id virtio_pci_id_table[] = {
 
 MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
 
-/* A PCI device has it's own struct device and so does a virtio device so
- * we create a place for the virtio devices to show up in sysfs.  I think it
- * would make more sense for virtio to not insist on having it's own device. */
-static struct device *virtio_pci_root;
-
 /* Convert a generic virtio device to our structure */
 static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
 {

@@ -629,7 +624,7 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
 	if (vp_dev == NULL)
 		return -ENOMEM;
 
-	vp_dev->vdev.dev.parent = virtio_pci_root;
+	vp_dev->vdev.dev.parent = &pci_dev->dev;
 	vp_dev->vdev.dev.release = virtio_pci_release_dev;
 	vp_dev->vdev.config = &virtio_pci_config_ops;
 	vp_dev->pci_dev = pci_dev;

@@ -717,17 +712,7 @@ static struct pci_driver virtio_pci_driver = {
 
 static int __init virtio_pci_init(void)
 {
-	int err;
-
-	virtio_pci_root = root_device_register("virtio-pci");
-	if (IS_ERR(virtio_pci_root))
-		return PTR_ERR(virtio_pci_root);
-
-	err = pci_register_driver(&virtio_pci_driver);
-	if (err)
-		root_device_unregister(virtio_pci_root);
-
-	return err;
+	return pci_register_driver(&virtio_pci_driver);
 }
 
 module_init(virtio_pci_init);

drivers/virtio/virtio_pci.c module_init(virtio_pci_init); · line 725

Quoted 7 lines

drivers/virtio/virtio_pci.c +0 -1

@@ -735,7 +720,6 @@ module_init(virtio_pci_init);
 static void __exit virtio_pci_exit(void)
 {
 	pci_unregister_driver(&virtio_pci_driver);
-	root_device_unregister(virtio_pci_root);
 }
 
 module_exit(virtio_pci_exit);

--
Gleb.

Quoted 13 lines · Milton Miller

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.

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Quoted 1 line

Signed-off-by Milton Miller <[email protected]>

Acked-by Gleb Natapov <[email protected]>

Quoted 58 lines

---

This is an alternative to the patch by Michael S. Tsirkin titled "virtio-pci: add softlinks between virtio and pci"
https://patchwork.kernel.org/patch/454581/

It creates simpler code, uses less memory, and should be even easier use by the installer as it won't have to know a virtio symlink to follow (just follow none).

Compile tested only as I don't have kvm setup.

drivers/virtio/virtio_pci.c +2 -17

index ef8d9d5..4fb5b2b 100644
@@ -96,11 +96,6 @@ static struct pci_device_id virtio_pci_id_table[] = {
 
 MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
 
-/* A PCI device has it's own struct device and so does a virtio device so
- * we create a place for the virtio devices to show up in sysfs.  I think it
- * would make more sense for virtio to not insist on having it's own device. */
-static struct device *virtio_pci_root;
-
 /* Convert a generic virtio device to our structure */
 static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
 {

@@ -629,7 +624,7 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
 	if (vp_dev == NULL)
 		return -ENOMEM;
 
-	vp_dev->vdev.dev.parent = virtio_pci_root;
+	vp_dev->vdev.dev.parent = &pci_dev->dev;
 	vp_dev->vdev.dev.release = virtio_pci_release_dev;
 	vp_dev->vdev.config = &virtio_pci_config_ops;
 	vp_dev->pci_dev = pci_dev;

@@ -717,17 +712,7 @@ static struct pci_driver virtio_pci_driver = {
 
 static int __init virtio_pci_init(void)
 {
-	int err;
-
-	virtio_pci_root = root_device_register("virtio-pci");
-	if (IS_ERR(virtio_pci_root))
-		return PTR_ERR(virtio_pci_root);
-
-	err = pci_register_driver(&virtio_pci_driver);
-	if (err)
-		root_device_unregister(virtio_pci_root);
-
-	return err;
+	return pci_register_driver(&virtio_pci_driver);
 }
 
 module_init(virtio_pci_init);

drivers/virtio/virtio_pci.c module_init(virtio_pci_init); · line 725

Quoted 7 lines

drivers/virtio/virtio_pci.c +0 -1

@@ -735,7 +720,6 @@ module_init(virtio_pci_init);
 static void __exit virtio_pci_exit(void)
 {
 	pci_unregister_driver(&virtio_pci_driver);
-	root_device_unregister(virtio_pci_root);
 }
 
 module_exit(virtio_pci_exit);

--
Gleb.

Quoted 14 lines · Gleb Natapov

Quoted 13 lines · Milton Miller

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.

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Quoted 2 lines

Quoted 1 line

Signed-off-by Milton Miller <[email protected]>

Acked-by Gleb Natapov <[email protected]>

Rusty, any comments?
We are going to tell the installer guys that this
is the interface, would be nice to know that the
patch is queued before we do.

Quoted 14 lines · Gleb Natapov

Quoted 13 lines · Milton Miller

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.

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Quoted 2 lines

Quoted 1 line

Signed-off-by Milton Miller <[email protected]>

Acked-by Gleb Natapov <[email protected]>

Rusty, any comments?
We are going to tell the installer guys that this
is the interface, would be nice to know that the
patch is queued before we do.

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.

Instead of creating a top-level devices/virtio-pci directory, create each device under the corresponding pci device node. Symlinks to all virtio-pci devices can be found under the pci driver link in bus/pci/drivers/virtio-pci/devices, and all virtio devices under drivers/bus/virtio/devices.

Signed-off-by Milton Miller <[email protected]>

---

This is an alternative to the patch by Michael S. Tsirkin titled "virtio-pci: add softlinks between virtio and pci"
https://patchwork.kernel.org/patch/454581/

It creates simpler code, uses less memory, and should be even easier use by the installer as it won't have to know a virtio symlink to follow (just follow none).

Compile tested only as I don't have kvm setup.

drivers/virtio/virtio_pci.c +2 -18

index ef8d9d5..4fb5b2b 100644
@@ -96,11 +96,6 @@ static struct pci_device_id virtio_pci_id_table[] = {
 
 MODULE_DEVICE_TABLE(pci, virtio_pci_id_table);
 
-/* A PCI device has it's own struct device and so does a virtio device so
- * we create a place for the virtio devices to show up in sysfs.  I think it
- * would make more sense for virtio to not insist on having it's own device. */
-static struct device *virtio_pci_root;
-
 /* Convert a generic virtio device to our structure */
 static struct virtio_pci_device *to_vp_device(struct virtio_device *vdev)
 {

@@ -629,7 +624,7 @@ static int __devinit virtio_pci_probe(struct pci_dev *pci_dev,
 	if (vp_dev == NULL)
 		return -ENOMEM;
 
-	vp_dev->vdev.dev.parent = virtio_pci_root;
+	vp_dev->vdev.dev.parent = &pci_dev->dev;
 	vp_dev->vdev.dev.release = virtio_pci_release_dev;
 	vp_dev->vdev.config = &virtio_pci_config_ops;
 	vp_dev->pci_dev = pci_dev;

@@ -717,17 +712,7 @@ static struct pci_driver virtio_pci_driver = {
 
 static int __init virtio_pci_init(void)
 {
-	int err;
-
-	virtio_pci_root = root_device_register("virtio-pci");
-	if (IS_ERR(virtio_pci_root))
-		return PTR_ERR(virtio_pci_root);
-
-	err = pci_register_driver(&virtio_pci_driver);
-	if (err)
-		root_device_unregister(virtio_pci_root);
-
-	return err;
+	return pci_register_driver(&virtio_pci_driver);
 }
 
 module_init(virtio_pci_init);

@@ -735,7 +720,6 @@ module_init(virtio_pci_init);
 static void __exit virtio_pci_exit(void)
 {
 	pci_unregister_driver(&virtio_pci_driver);
-	root_device_unregister(virtio_pci_root);
 }
 
 module_exit(virtio_pci_exit);