kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 01/12] kvm: qemu: device-assignment: add_assigned_device() returns a pointer
@ 2008-11-28 17:10 Mark McLoughlin
  2008-11-28 17:10 ` [PATCH 02/12] kvm: qemu: device-assignment: introduce add_assigned_devices() Mark McLoughlin
  2008-11-28 17:14 ` [PATCH 01/12] kvm: qemu: device-assignment: add_assigned_device() returns a pointer Mark McLoughlin
  0 siblings, 2 replies; 18+ messages in thread
From: Mark McLoughlin @ 2008-11-28 17:10 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm, Mark McLoughlin

add_assigned_device() returns a pointer, so don't check the return
value is less than zero.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 qemu/hw/ipf.c |    2 +-
 qemu/hw/pc.c  |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/qemu/hw/ipf.c b/qemu/hw/ipf.c
index 37f2de7..c0ac9eb 100644
--- a/qemu/hw/ipf.c
+++ b/qemu/hw/ipf.c
@@ -650,7 +650,7 @@ static void ipf_init1(ram_addr_t ram_size, int vga_ram_size,
     if (kvm_enabled()) {
 	int i;
         for (i = 0; i < assigned_devices_index; i++) {
-            if (add_assigned_device(assigned_devices[i]) < 0) {
+            if (!add_assigned_device(assigned_devices[i])) {
                 fprintf(stderr, "Warning: could not add assigned device %s\n",
                         assigned_devices[i]);
             }
diff --git a/qemu/hw/pc.c b/qemu/hw/pc.c
index 7d296c4..6f3339f 100644
--- a/qemu/hw/pc.c
+++ b/qemu/hw/pc.c
@@ -1187,7 +1187,7 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
     if (kvm_enabled()) {
 	int i;
         for (i = 0; i < assigned_devices_index; i++) {
-            if (add_assigned_device(assigned_devices[i]) < 0) {
+            if (!add_assigned_device(assigned_devices[i])) {
                 fprintf(stderr, "Warning: could not add assigned device %s\n",
                         assigned_devices[i]);
             }
-- 
1.5.4.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 02/12] kvm: qemu: device-assignment: introduce add_assigned_devices()
  2008-11-28 17:10 [PATCH 01/12] kvm: qemu: device-assignment: add_assigned_device() returns a pointer Mark McLoughlin
@ 2008-11-28 17:10 ` Mark McLoughlin
  2008-11-28 17:10   ` [PATCH 03/12] kvm: qemu: device-assignment: exit if cmdline parsing fails Mark McLoughlin
  2008-11-28 17:14 ` [PATCH 01/12] kvm: qemu: device-assignment: add_assigned_device() returns a pointer Mark McLoughlin
  1 sibling, 1 reply; 18+ messages in thread
From: Mark McLoughlin @ 2008-11-28 17:10 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm, Mark McLoughlin

Merge two copies of the same code and cleanup with no functional
changes.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>

fix error messages

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 qemu/hw/device-assignment.c |   32 +++++++++++++++++++++-----------
 qemu/hw/device-assignment.h |    2 +-
 qemu/hw/ipf.c               |   16 ++--------------
 qemu/hw/pc.c                |   16 ++--------------
 4 files changed, 26 insertions(+), 40 deletions(-)

diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c
index 9a790c6..eb2a73a 100644
--- a/qemu/hw/device-assignment.c
+++ b/qemu/hw/device-assignment.c
@@ -554,17 +554,6 @@ struct PCIDevice *init_assigned_device(AssignedDevInfo *adev, PCIBus *bus)
     return &dev->dev;
 }
 
-int init_all_assigned_devices(PCIBus *bus)
-{
-    struct AssignedDevInfo *adev;
-
-    LIST_FOREACH(adev, &adev_head, next)
-        if (init_assigned_device(adev, bus) == NULL)
-            return -1;
-
-    return 0;
-}
-
 /*
  * Syntax to assign device:
  *
@@ -619,3 +608,24 @@ bad:
     qemu_free(adev);
     return NULL;
 }
+
+void add_assigned_devices(PCIBus *bus, const char **devices, int n_devices)
+{
+    int i;
+
+    for (i = 0; i < n_devices; i++) {
+        struct AssignedDevInfo *adev;
+
+        adev = add_assigned_device(devices[i]);
+        if (!adev) {
+            fprintf(stderr, "Could not add assigned device %s\n", devices[i]);
+            continue;
+        }
+
+        if (!init_assigned_device(adev, bus)) {
+            fprintf(stderr, "Failed to initialize assigned device %s\n",
+                    devices[i]);
+            exit(1);
+        }
+    }
+}
diff --git a/qemu/hw/device-assignment.h b/qemu/hw/device-assignment.h
index d6caa67..5a01d98 100644
--- a/qemu/hw/device-assignment.h
+++ b/qemu/hw/device-assignment.h
@@ -96,7 +96,7 @@ struct AssignedDevInfo {
 
 PCIDevice *init_assigned_device(AssignedDevInfo *adev, PCIBus *bus);
 AssignedDevInfo *add_assigned_device(const char *arg);
-int init_all_assigned_devices(PCIBus *bus);
+void add_assigned_devices(PCIBus *bus, const char **devices, int n_devices);
 
 #define MAX_DEV_ASSIGN_CMDLINE 8
 
diff --git a/qemu/hw/ipf.c b/qemu/hw/ipf.c
index c0ac9eb..3e24c98 100644
--- a/qemu/hw/ipf.c
+++ b/qemu/hw/ipf.c
@@ -647,20 +647,8 @@ static void ipf_init1(ram_addr_t ram_size, int vga_ram_size,
     }
 
 #ifdef USE_KVM_DEVICE_ASSIGNMENT
-    if (kvm_enabled()) {
-	int i;
-        for (i = 0; i < assigned_devices_index; i++) {
-            if (!add_assigned_device(assigned_devices[i])) {
-                fprintf(stderr, "Warning: could not add assigned device %s\n",
-                        assigned_devices[i]);
-            }
-        }
-
-	if (init_all_assigned_devices(pci_bus)) {
-	    fprintf(stderr, "Failed to initialize assigned devices\n");
-	    exit (1);
-	}
-    }
+    if (kvm_enabled())
+	add_assigned_devices(pci_bus, assigned_devices, assigned_devices_index);
 #endif /* USE_KVM_DEVICE_ASSIGNMENT */
 
 }
diff --git a/qemu/hw/pc.c b/qemu/hw/pc.c
index 6f3339f..6de460c 100644
--- a/qemu/hw/pc.c
+++ b/qemu/hw/pc.c
@@ -1184,20 +1184,8 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
         virtio_balloon_init(pci_bus);
 
 #ifdef USE_KVM_DEVICE_ASSIGNMENT
-    if (kvm_enabled()) {
-	int i;
-        for (i = 0; i < assigned_devices_index; i++) {
-            if (!add_assigned_device(assigned_devices[i])) {
-                fprintf(stderr, "Warning: could not add assigned device %s\n",
-                        assigned_devices[i]);
-            }
-        }
-
-	if (init_all_assigned_devices(pci_bus)) {
-	    fprintf(stderr, "Failed to initialize assigned devices\n");
-	    exit (1);
-	}
-    }
+    if (kvm_enabled())
+	add_assigned_devices(pci_bus, assigned_devices, assigned_devices_index);
 #endif /* USE_KVM_DEVICE_ASSIGNMENT */
 }
 
-- 
1.5.4.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 03/12] kvm: qemu: device-assignment: exit if cmdline parsing fails
  2008-11-28 17:10 ` [PATCH 02/12] kvm: qemu: device-assignment: introduce add_assigned_devices() Mark McLoughlin
@ 2008-11-28 17:10   ` Mark McLoughlin
  2008-11-28 17:10     ` [PATCH 04/12] kvm: qemu: device-assignment: unregister device if assignment fails Mark McLoughlin
  2008-12-10 10:23     ` [PATCH] kvm: qemu: device-assignment: really exit if cmdline parsing fails Mark McLoughlin
  0 siblings, 2 replies; 18+ messages in thread
From: Mark McLoughlin @ 2008-11-28 17:10 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm, Mark McLoughlin

It's standard practice in qemu to exit if command line parameter
fails, so do that here too.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 qemu/hw/device-assignment.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c
index eb2a73a..8fbd66c 100644
--- a/qemu/hw/device-assignment.c
+++ b/qemu/hw/device-assignment.c
@@ -620,6 +620,7 @@ void add_assigned_devices(PCIBus *bus, const char **devices, int n_devices)
         if (!adev) {
             fprintf(stderr, "Could not add assigned device %s\n", devices[i]);
             continue;
+            exit(1);
         }
 
         if (!init_assigned_device(adev, bus)) {
-- 
1.5.4.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 04/12] kvm: qemu: device-assignment: unregister device if assignment fails
  2008-11-28 17:10   ` [PATCH 03/12] kvm: qemu: device-assignment: exit if cmdline parsing fails Mark McLoughlin
@ 2008-11-28 17:10     ` Mark McLoughlin
  2008-11-28 17:10       ` [PATCH 05/12] kvm: qemu: device-assignment: fixup error reporting Mark McLoughlin
  2008-12-10 10:23     ` [PATCH] kvm: qemu: device-assignment: really exit if cmdline parsing fails Mark McLoughlin
  1 sibling, 1 reply; 18+ messages in thread
From: Mark McLoughlin @ 2008-11-28 17:10 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm, Mark McLoughlin

Fixes a segfault in assigned_dev_update_irq() if assigning a device
previously failed.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 qemu/hw/device-assignment.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c
index 8fbd66c..b5cf6ee 100644
--- a/qemu/hw/device-assignment.c
+++ b/qemu/hw/device-assignment.c
@@ -550,8 +550,11 @@ struct PCIDevice *init_assigned_device(AssignedDevInfo *adev, PCIBus *bus)
     }
 
     adev->assigned_dev = dev;
-  out:
     return &dev->dev;
+
+out:
+    pci_unregister_device(&dev->dev);
+    return NULL;
 }
 
 /*
-- 
1.5.4.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 05/12] kvm: qemu: device-assignment: fixup error reporting
  2008-11-28 17:10     ` [PATCH 04/12] kvm: qemu: device-assignment: unregister device if assignment fails Mark McLoughlin
@ 2008-11-28 17:10       ` Mark McLoughlin
  2008-11-28 17:10         ` [PATCH 06/12] kvm: qemu: device-assignment: cleanup irq assignment error messages Mark McLoughlin
  0 siblings, 1 reply; 18+ messages in thread
From: Mark McLoughlin @ 2008-11-28 17:10 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm, Mark McLoughlin

If kvm_assign_pci_device(), there's no need for us to print
two lines of error messages. The string translation of errno
is very useful though, so include that in the message using
strerror().

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 qemu/hw/device-assignment.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c
index b5cf6ee..2b2ef68 100644
--- a/qemu/hw/device-assignment.c
+++ b/qemu/hw/device-assignment.c
@@ -540,12 +540,11 @@ struct PCIDevice *init_assigned_device(AssignedDevInfo *adev, PCIBus *bus)
     if (r && !adev->disable_iommu)
 	assigned_dev_data.flags |= KVM_DEV_ASSIGN_ENABLE_IOMMU;
 #endif
-      
+
     r = kvm_assign_pci_device(kvm_context, &assigned_dev_data);
     if (r < 0) {
-	fprintf(stderr, "Could not notify kernel about "
-                "assigned device \"%s\"\n", adev->name);
-	perror("register_real_device");
+	fprintf(stderr, "Failed to assign device \"%s\" : %s\n",
+                adev->name, strerror(-r));
 	goto out;
     }
 
-- 
1.5.4.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 06/12] kvm: qemu: device-assignment: cleanup irq assignment error messages
  2008-11-28 17:10       ` [PATCH 05/12] kvm: qemu: device-assignment: fixup error reporting Mark McLoughlin
@ 2008-11-28 17:10         ` Mark McLoughlin
  2008-11-28 17:10           ` [PATCH 07/12] kvm: qemu: device-assignment: remove device if irq assignment fails Mark McLoughlin
  2008-11-28 19:05           ` [PATCH 06/12] kvm: qemu: device-assignment: cleanup irq assignment error messages John Rousseau
  0 siblings, 2 replies; 18+ messages in thread
From: Mark McLoughlin @ 2008-11-28 17:10 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm, Mark McLoughlin

Replace perror() usage with sane error message.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 qemu/hw/device-assignment.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c
index 2b2ef68..b39617a 100644
--- a/qemu/hw/device-assignment.c
+++ b/qemu/hw/device-assignment.c
@@ -473,9 +473,10 @@ void assigned_dev_update_irq(PCIDevice *d)
             assigned_irq_data.host_irq = assigned_dev->real_device.irq;
             r = kvm_assign_irq(kvm_context, &assigned_irq_data);
             if (r < 0) {
-                perror("assigned_dev_update_irq");
-                fprintf(stderr, "Are you assigning a device "
-                        "that shares IRQ with some other device?\n");
+                fprintf(stderr, "Failed to assign irq for \"%s\": %s\n",
+                        adev->name, strerror(-r));
+                fprintf(stderr, "Perhaps you re you assigning a device "
+                        "that shares IRQ with another device?\n");
                 pci_unregister_device(&assigned_dev->dev);
                 /* FIXME: Delete node from list */
                 continue;
-- 
1.5.4.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 07/12] kvm: qemu: device-assignment: remove device if irq assignment fails
  2008-11-28 17:10         ` [PATCH 06/12] kvm: qemu: device-assignment: cleanup irq assignment error messages Mark McLoughlin
@ 2008-11-28 17:10           ` Mark McLoughlin
  2008-11-28 17:10             ` [PATCH 08/12] kvm: qemu: device-assignment: introduce free_assigned_device() Mark McLoughlin
  2008-11-28 19:05           ` [PATCH 06/12] kvm: qemu: device-assignment: cleanup irq assignment error messages John Rousseau
  1 sibling, 1 reply; 18+ messages in thread
From: Mark McLoughlin @ 2008-11-28 17:10 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm, Mark McLoughlin

Fix blatant issue where we leave a freed assigned device node on
the list if irq assignment fails.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 qemu/hw/device-assignment.c |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c
index b39617a..fde17ac 100644
--- a/qemu/hw/device-assignment.c
+++ b/qemu/hw/device-assignment.c
@@ -449,12 +449,14 @@ static uint32_t calc_assigned_dev_id(uint8_t bus, uint8_t devfn)
  */
 void assigned_dev_update_irq(PCIDevice *d)
 {
-    int irq, r;
-    AssignedDevice *assigned_dev;
     AssignedDevInfo *adev;
 
-    LIST_FOREACH(adev, &adev_head, next) {
-        assigned_dev = adev->assigned_dev;
+    adev = LIST_FIRST(&adev_head);
+    while (adev) {
+        AssignedDevInfo *next = LIST_NEXT(adev, next);
+        AssignedDevice *assigned_dev = adev->assigned_dev;
+        int irq, r;
+
         irq = pci_map_irq(&assigned_dev->dev, assigned_dev->intpin);
         irq = piix_get_irq(irq);
 
@@ -477,12 +479,15 @@ void assigned_dev_update_irq(PCIDevice *d)
                         adev->name, strerror(-r));
                 fprintf(stderr, "Perhaps you re you assigning a device "
                         "that shares IRQ with another device?\n");
+                LIST_REMOVE(adev, next);
                 pci_unregister_device(&assigned_dev->dev);
-                /* FIXME: Delete node from list */
+                adev = next;
                 continue;
             }
             assigned_dev->girq = irq;
         }
+
+        adev = next;
     }
 }
 
-- 
1.5.4.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 08/12] kvm: qemu: device-assignment: introduce free_assigned_device()
  2008-11-28 17:10           ` [PATCH 07/12] kvm: qemu: device-assignment: remove device if irq assignment fails Mark McLoughlin
@ 2008-11-28 17:10             ` Mark McLoughlin
  2008-11-28 17:10               ` [PATCH 09/12] kvm: qemu: device-assignment: free device if hotplug fails Mark McLoughlin
  0 siblings, 1 reply; 18+ messages in thread
From: Mark McLoughlin @ 2008-11-28 17:10 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm, Mark McLoughlin

Add a function to fully free the AssignedDevice and AssignedDevInfo
and remove from the list.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 qemu/hw/device-assignment.c |   16 ++++++++++++++--
 qemu/hw/device-assignment.h |    1 +
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c
index fde17ac..2836059 100644
--- a/qemu/hw/device-assignment.c
+++ b/qemu/hw/device-assignment.c
@@ -439,6 +439,19 @@ again:
 
 static LIST_HEAD(, AssignedDevInfo) adev_head;
 
+void free_assigned_device(AssignedDevInfo *adev)
+{
+    AssignedDevice *dev = adev->assigned_dev;
+
+    if (dev) {
+        pci_unregister_device(&dev->dev);
+        adev->assigned_dev = dev = NULL;
+    }
+
+    LIST_REMOVE(adev, next);
+    qemu_free(adev);
+}
+
 static uint32_t calc_assigned_dev_id(uint8_t bus, uint8_t devfn)
 {
     return (uint32_t)bus << 8 | (uint32_t)devfn;
@@ -479,8 +492,7 @@ void assigned_dev_update_irq(PCIDevice *d)
                         adev->name, strerror(-r));
                 fprintf(stderr, "Perhaps you re you assigning a device "
                         "that shares IRQ with another device?\n");
-                LIST_REMOVE(adev, next);
-                pci_unregister_device(&assigned_dev->dev);
+                free_assigned_device(adev);
                 adev = next;
                 continue;
             }
diff --git a/qemu/hw/device-assignment.h b/qemu/hw/device-assignment.h
index 5a01d98..c8c47d3 100644
--- a/qemu/hw/device-assignment.h
+++ b/qemu/hw/device-assignment.h
@@ -94,6 +94,7 @@ struct AssignedDevInfo {
     int disable_iommu;
 };
 
+void free_assigned_device(AssignedDevInfo *adev);
 PCIDevice *init_assigned_device(AssignedDevInfo *adev, PCIBus *bus);
 AssignedDevInfo *add_assigned_device(const char *arg);
 void add_assigned_devices(PCIBus *bus, const char **devices, int n_devices);
-- 
1.5.4.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 09/12] kvm: qemu: device-assignment: free device if hotplug fails
  2008-11-28 17:10             ` [PATCH 08/12] kvm: qemu: device-assignment: introduce free_assigned_device() Mark McLoughlin
@ 2008-11-28 17:10               ` Mark McLoughlin
  2008-11-28 17:10                 ` [PATCH 10/12] kvm: qemu: device-assignment: close PCIDevRegions::config_fd Mark McLoughlin
  0 siblings, 1 reply; 18+ messages in thread
From: Mark McLoughlin @ 2008-11-28 17:10 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm, Mark McLoughlin

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 qemu/hw/device-hotplug.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/qemu/hw/device-hotplug.c b/qemu/hw/device-hotplug.c
index ba1b161..0bcac60 100644
--- a/qemu/hw/device-hotplug.c
+++ b/qemu/hw/device-hotplug.c
@@ -48,6 +48,11 @@ static PCIDevice *qemu_system_hot_assign_device(const char *opts, int bus_nr)
     }
  
     ret = init_assigned_device(adev, pci_bus);
+    if (ret == NULL) {
+        term_printf("Failed to assign device\n");
+        free_assigned_device(adev);
+        return NULL;
+    }
 
     term_printf("Registered host PCI device %02x:%02x.%1x "
 		"(\"%s\") as guest device %02x:%02x.%1x\n",
-- 
1.5.4.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 10/12] kvm: qemu: device-assignment: close PCIDevRegions::config_fd
  2008-11-28 17:10               ` [PATCH 09/12] kvm: qemu: device-assignment: free device if hotplug fails Mark McLoughlin
@ 2008-11-28 17:10                 ` Mark McLoughlin
  2008-11-28 17:10                   ` [PATCH 11/12] kvm: qemu: device-assignment: munmap() mmio regions Mark McLoughlin
  0 siblings, 1 reply; 18+ messages in thread
From: Mark McLoughlin @ 2008-11-28 17:10 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm, Mark McLoughlin

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 qemu/hw/device-assignment.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c
index 2836059..5f803b1 100644
--- a/qemu/hw/device-assignment.c
+++ b/qemu/hw/device-assignment.c
@@ -444,6 +444,11 @@ void free_assigned_device(AssignedDevInfo *adev)
     AssignedDevice *dev = adev->assigned_dev;
 
     if (dev) {
+        if (dev->real_device.config_fd) {
+            close(dev->real_device.config_fd);
+            dev->real_device.config_fd = 0;
+        }
+
         pci_unregister_device(&dev->dev);
         adev->assigned_dev = dev = NULL;
     }
-- 
1.5.4.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 11/12] kvm: qemu: device-assignment: munmap() mmio regions
  2008-11-28 17:10                 ` [PATCH 10/12] kvm: qemu: device-assignment: close PCIDevRegions::config_fd Mark McLoughlin
@ 2008-11-28 17:10                   ` Mark McLoughlin
  2008-11-28 17:10                     ` [PATCH 12/12] kvm: qemu: device-assignment: init_assigned_device() error handling Mark McLoughlin
  0 siblings, 1 reply; 18+ messages in thread
From: Mark McLoughlin @ 2008-11-28 17:10 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm, Mark McLoughlin

Undo the effects of assigned_dev_register_regions(); made
unneccessarily difficult by the twisty data structures.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 qemu/hw/device-assignment.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c
index 5f803b1..1964fbc 100644
--- a/qemu/hw/device-assignment.c
+++ b/qemu/hw/device-assignment.c
@@ -330,6 +330,7 @@ static int assigned_dev_register_regions(PCIRegion *io_regions,
                      cur_region->resource_fd, (off_t) 0);
 
             if (pci_dev->v_addrs[i].u.r_virtbase == MAP_FAILED) {
+                pci_dev->v_addrs[i].u.r_virtbase = NULL;
                 fprintf(stderr, "%s: Error: Couldn't mmap 0x%x!"
                         "\n", __func__,
                         (uint32_t) (cur_region->base_addr));
@@ -444,6 +445,25 @@ void free_assigned_device(AssignedDevInfo *adev)
     AssignedDevice *dev = adev->assigned_dev;
 
     if (dev) {
+        int i;
+
+        for (i = 0; i < dev->real_device.region_number; i++) {
+            PCIRegion *pci_region = &dev->real_device.regions[i];
+            AssignedDevRegion *region = &dev->v_addrs[i];
+
+            if (!pci_region->valid || !(pci_region->type & IORESOURCE_MEM))
+                continue;
+
+            if (region->u.r_virtbase) {
+                int ret = munmap(region->u.r_virtbase,
+                                 (pci_region->size + 0xFFF) & 0xFFFFF000);
+                if (ret != 0)
+                    fprintf(stderr,
+                            "Failed to unmap assigned device region: %s\n",
+                            strerror(errno));
+            }
+        }
+
         if (dev->real_device.config_fd) {
             close(dev->real_device.config_fd);
             dev->real_device.config_fd = 0;
-- 
1.5.4.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 12/12] kvm: qemu: device-assignment: init_assigned_device() error handling
  2008-11-28 17:10                   ` [PATCH 11/12] kvm: qemu: device-assignment: munmap() mmio regions Mark McLoughlin
@ 2008-11-28 17:10                     ` Mark McLoughlin
  0 siblings, 0 replies; 18+ messages in thread
From: Mark McLoughlin @ 2008-11-28 17:10 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm, Mark McLoughlin

Associate the AssignedDevice with the AssignedDevInfo as soon as
we have allocated it and don't try and free it in init_assigned_device()
if an error occurs.

This ensures that the mmio region is unmapped by free_assigned_device()
if init_assigned_device() fails.

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 qemu/hw/device-assignment.c |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c
index 1964fbc..70640c9 100644
--- a/qemu/hw/device-assignment.c
+++ b/qemu/hw/device-assignment.c
@@ -548,17 +548,19 @@ struct PCIDevice *init_assigned_device(AssignedDevInfo *adev, PCIBus *bus)
         return NULL;
     }
 
+    adev->assigned_dev = dev;
+
     if (get_real_device(dev, adev->bus, adev->dev, adev->func)) {
         fprintf(stderr, "%s: Error: Couldn't get real device (%s)!\n",
                 __func__, adev->name);
-        goto out;
+        return NULL;
     }
 
     /* handle real device's MMIO/PIO BARs */
     if (assigned_dev_register_regions(dev->real_device.regions,
                                       dev->real_device.region_number,
                                       dev))
-        goto out;
+        return NULL;
 
     /* handle interrupt routing */
     e_device = (dev->dev.devfn >> 3) & 0x1f;
@@ -588,15 +590,10 @@ struct PCIDevice *init_assigned_device(AssignedDevInfo *adev, PCIBus *bus)
     if (r < 0) {
 	fprintf(stderr, "Failed to assign device \"%s\" : %s\n",
                 adev->name, strerror(-r));
-	goto out;
+	return NULL;
     }
 
-    adev->assigned_dev = dev;
     return &dev->dev;
-
-out:
-    pci_unregister_device(&dev->dev);
-    return NULL;
 }
 
 /*
-- 
1.5.4.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 01/12] kvm: qemu: device-assignment: add_assigned_device() returns a pointer
  2008-11-28 17:10 [PATCH 01/12] kvm: qemu: device-assignment: add_assigned_device() returns a pointer Mark McLoughlin
  2008-11-28 17:10 ` [PATCH 02/12] kvm: qemu: device-assignment: introduce add_assigned_devices() Mark McLoughlin
@ 2008-11-28 17:14 ` Mark McLoughlin
  2008-11-30 10:42   ` Avi Kivity
  1 sibling, 1 reply; 18+ messages in thread
From: Mark McLoughlin @ 2008-11-28 17:14 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm

Hi Avi,

Sorry ... forgot to 'git send-email --compose' :-)

The patchset is just a bunch of fixes to the device-assignment error
handling code. Shouldn't be anything too controversial there.

Cheers,
Mark.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 06/12] kvm: qemu: device-assignment: cleanup irq assignment error messages
  2008-11-28 17:10         ` [PATCH 06/12] kvm: qemu: device-assignment: cleanup irq assignment error messages Mark McLoughlin
  2008-11-28 17:10           ` [PATCH 07/12] kvm: qemu: device-assignment: remove device if irq assignment fails Mark McLoughlin
@ 2008-11-28 19:05           ` John Rousseau
  2008-11-30 10:47             ` Avi Kivity
  1 sibling, 1 reply; 18+ messages in thread
From: John Rousseau @ 2008-11-28 19:05 UTC (permalink / raw)
  To: Mark McLoughlin; +Cc: Avi Kivity, kvm

Mark McLoughlin wrote:
> Replace perror() usage with sane error message.
> 
> Signed-off-by: Mark McLoughlin <markmc@redhat.com>
> ---
>  qemu/hw/device-assignment.c |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c
> index 2b2ef68..b39617a 100644
> --- a/qemu/hw/device-assignment.c
> +++ b/qemu/hw/device-assignment.c
> @@ -473,9 +473,10 @@ void assigned_dev_update_irq(PCIDevice *d)
>              assigned_irq_data.host_irq = assigned_dev->real_device.irq;
>              r = kvm_assign_irq(kvm_context, &assigned_irq_data);
>              if (r < 0) {
> -                perror("assigned_dev_update_irq");
> -                fprintf(stderr, "Are you assigning a device "
> -                        "that shares IRQ with some other device?\n");
> +                fprintf(stderr, "Failed to assign irq for \"%s\": %s\n",
> +                        adev->name, strerror(-r));
> +                fprintf(stderr, "Perhaps you re you assigning a device "

Typo.

> +                        "that shares IRQ with another device?\n");
>                  pci_unregister_device(&assigned_dev->dev);
>                  /* FIXME: Delete node from list */
>                  continue;

-John

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 01/12] kvm: qemu: device-assignment: add_assigned_device() returns a pointer
  2008-11-28 17:14 ` [PATCH 01/12] kvm: qemu: device-assignment: add_assigned_device() returns a pointer Mark McLoughlin
@ 2008-11-30 10:42   ` Avi Kivity
  0 siblings, 0 replies; 18+ messages in thread
From: Avi Kivity @ 2008-11-30 10:42 UTC (permalink / raw)
  To: Mark McLoughlin; +Cc: kvm

Mark McLoughlin wrote:
> Hi Avi,
>
> Sorry ... forgot to 'git send-email --compose' :-)
>
> The patchset is just a bunch of fixes to the device-assignment error
> handling code. Shouldn't be anything too controversial there.
>   

No, all applied.  Patch 2 looks like two patches munged together; I 
applied it anyway as it wasn't worth the effort to respin.

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 06/12] kvm: qemu: device-assignment: cleanup irq assignment error messages
  2008-11-28 19:05           ` [PATCH 06/12] kvm: qemu: device-assignment: cleanup irq assignment error messages John Rousseau
@ 2008-11-30 10:47             ` Avi Kivity
  0 siblings, 0 replies; 18+ messages in thread
From: Avi Kivity @ 2008-11-30 10:47 UTC (permalink / raw)
  To: John Rousseau; +Cc: Mark McLoughlin, kvm

John Rousseau wrote:
>> -                perror("assigned_dev_update_irq");
>> -                fprintf(stderr, "Are you assigning a device "
>> -                        "that shares IRQ with some other device?\n");
>> +                fprintf(stderr, "Failed to assign irq for \"%s\": 
>> %s\n",
>> +                        adev->name, strerror(-r));
>> +                fprintf(stderr, "Perhaps you re you assigning a 
>> device "
>
> Typo.

Thanks, updated.

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH] kvm: qemu: device-assignment: really exit if cmdline parsing fails
  2008-11-28 17:10   ` [PATCH 03/12] kvm: qemu: device-assignment: exit if cmdline parsing fails Mark McLoughlin
  2008-11-28 17:10     ` [PATCH 04/12] kvm: qemu: device-assignment: unregister device if assignment fails Mark McLoughlin
@ 2008-12-10 10:23     ` Mark McLoughlin
  2008-12-10 10:28       ` Avi Kivity
  1 sibling, 1 reply; 18+ messages in thread
From: Mark McLoughlin @ 2008-12-10 10:23 UTC (permalink / raw)
  To: Avi Kivity; +Cc: kvm

On Fri, 2008-11-28 at 17:10 +0000, Mark McLoughlin wrote:
> It's standard practice in qemu to exit if command line parameter
> fails, so do that here too.
> 
> Signed-off-by: Mark McLoughlin <markmc@redhat.com>
> ---
>  qemu/hw/device-assignment.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c
> index eb2a73a..8fbd66c 100644
> --- a/qemu/hw/device-assignment.c
> +++ b/qemu/hw/device-assignment.c
> @@ -620,6 +620,7 @@ void add_assigned_devices(PCIBus *bus, const char **devices, int n_devices)
>          if (!adev) {
>              fprintf(stderr, "Could not add assigned device %s\n", devices[i]);
>              continue;
> +            exit(1);

Um, that's a rather embarrassing thinko.

Cheers,
Mark.

From: Mark McLoughlin <markmc@redhat.com>
Subject: [PATCH] kvm: qemu: device-assignment: really exit if cmdline parsing fails

Signed-off-by: Mark McLoughlin <markmc@redhat.com>
---
 qemu/hw/device-assignment.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c
index 4a38a22..7a66665 100644
--- a/qemu/hw/device-assignment.c
+++ b/qemu/hw/device-assignment.c
@@ -668,7 +668,6 @@ void add_assigned_devices(PCIBus *bus, const char **devices, int n_devices)
         adev = add_assigned_device(devices[i]);
         if (!adev) {
             fprintf(stderr, "Could not add assigned device %s\n", devices[i]);
-            continue;
             exit(1);
         }
 
-- 
1.5.4.3


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] kvm: qemu: device-assignment: really exit if cmdline parsing fails
  2008-12-10 10:23     ` [PATCH] kvm: qemu: device-assignment: really exit if cmdline parsing fails Mark McLoughlin
@ 2008-12-10 10:28       ` Avi Kivity
  0 siblings, 0 replies; 18+ messages in thread
From: Avi Kivity @ 2008-12-10 10:28 UTC (permalink / raw)
  To: Mark McLoughlin; +Cc: kvm

Mark McLoughlin wrote:
>>              fprintf(stderr, "Could not add assigned device %s\n", devices[i]);
>>              continue;
>> +            exit(1);
>>     
>
> Um, that's a rather embarrassing thinko.
>   

For the reviewer as well; applied.

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2008-12-10 10:28 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-11-28 17:10 [PATCH 01/12] kvm: qemu: device-assignment: add_assigned_device() returns a pointer Mark McLoughlin
2008-11-28 17:10 ` [PATCH 02/12] kvm: qemu: device-assignment: introduce add_assigned_devices() Mark McLoughlin
2008-11-28 17:10   ` [PATCH 03/12] kvm: qemu: device-assignment: exit if cmdline parsing fails Mark McLoughlin
2008-11-28 17:10     ` [PATCH 04/12] kvm: qemu: device-assignment: unregister device if assignment fails Mark McLoughlin
2008-11-28 17:10       ` [PATCH 05/12] kvm: qemu: device-assignment: fixup error reporting Mark McLoughlin
2008-11-28 17:10         ` [PATCH 06/12] kvm: qemu: device-assignment: cleanup irq assignment error messages Mark McLoughlin
2008-11-28 17:10           ` [PATCH 07/12] kvm: qemu: device-assignment: remove device if irq assignment fails Mark McLoughlin
2008-11-28 17:10             ` [PATCH 08/12] kvm: qemu: device-assignment: introduce free_assigned_device() Mark McLoughlin
2008-11-28 17:10               ` [PATCH 09/12] kvm: qemu: device-assignment: free device if hotplug fails Mark McLoughlin
2008-11-28 17:10                 ` [PATCH 10/12] kvm: qemu: device-assignment: close PCIDevRegions::config_fd Mark McLoughlin
2008-11-28 17:10                   ` [PATCH 11/12] kvm: qemu: device-assignment: munmap() mmio regions Mark McLoughlin
2008-11-28 17:10                     ` [PATCH 12/12] kvm: qemu: device-assignment: init_assigned_device() error handling Mark McLoughlin
2008-11-28 19:05           ` [PATCH 06/12] kvm: qemu: device-assignment: cleanup irq assignment error messages John Rousseau
2008-11-30 10:47             ` Avi Kivity
2008-12-10 10:23     ` [PATCH] kvm: qemu: device-assignment: really exit if cmdline parsing fails Mark McLoughlin
2008-12-10 10:28       ` Avi Kivity
2008-11-28 17:14 ` [PATCH 01/12] kvm: qemu: device-assignment: add_assigned_device() returns a pointer Mark McLoughlin
2008-11-30 10:42   ` Avi Kivity

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).