diff mbox

[2/2] kvm: qemu: check device assignment command

Message ID 715D42877B251141A38726ABF5CABF2C01A11B66EE@pdsmsx503.ccr.corp.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Han, Weidong March 26, 2009, 9:02 a.m. UTC
Device assignment command is like "-pcidevice host=xx:yy.z".
Check bus:dev.func length to make sure its format is xx:yy.z.

Signed-off-by: Weidong Han <weidong.han@intel.com>
---
 qemu/hw/device-assignment.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

Comments

Avi Kivity March 26, 2009, 12:04 p.m. UTC | #1
Han, Weidong wrote:
> Device assignment command is like "-pcidevice host=xx:yy.z".
> Check bus:dev.func length to make sure its format is xx:yy.z.
>
> Signed-off-by: Weidong Han <weidong.han@intel.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 cef7c8a..50a0d5c 100644
> --- a/qemu/hw/device-assignment.c
> +++ b/qemu/hw/device-assignment.c
> @@ -1196,7 +1196,7 @@ out:
>  AssignedDevInfo *add_assigned_device(const char *arg)
>  {
>      char *cp, *cp1;
> -    char device[8];
> +    char device[9];
>      char dma[6];
>      int r;
>      AssignedDevInfo *adev;
> @@ -1207,6 +1207,9 @@ AssignedDevInfo *add_assigned_device(const char *arg)
>          return NULL;
>      }
>      r = get_param_value(device, sizeof(device), "host", arg);
> +    /* b:d.f format: xx:yy.z */
> +    if (r != 7)
> +        goto bad;
>      r = get_param_value(adev->name, sizeof(adev->name), "name", arg);
>      if (!r)
>  	snprintf(adev->name, sizeof(adev->name), "%s", device);
>   

I suggest replacing the parsing code with pci_parse_devaddr() (needs to 
be extended to support functions) so that all the checking and parsing 
is done in one place.
Han, Weidong March 27, 2009, 9:31 a.m. UTC | #2
Avi Kivity wrote:
> Han, Weidong wrote:
>> Device assignment command is like "-pcidevice host=xx:yy.z".
>> Check bus:dev.func length to make sure its format is xx:yy.z.
>> 
>> Signed-off-by: Weidong Han <weidong.han@intel.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 cef7c8a..50a0d5c 100644 ---
>> a/qemu/hw/device-assignment.c +++ b/qemu/hw/device-assignment.c
>> @@ -1196,7 +1196,7 @@ out:
>>  AssignedDevInfo *add_assigned_device(const char *arg)  {
>>      char *cp, *cp1;
>> -    char device[8];
>> +    char device[9];
>>      char dma[6];
>>      int r;
>>      AssignedDevInfo *adev;
>> @@ -1207,6 +1207,9 @@ AssignedDevInfo *add_assigned_device(const
>>      char *arg)          return NULL; }
>>      r = get_param_value(device, sizeof(device), "host", arg);
>> +    /* b:d.f format: xx:yy.z */
>> +    if (r != 7)
>> +        goto bad;
>>      r = get_param_value(adev->name, sizeof(adev->name), "name",
>>  	arg);      if (!r) snprintf(adev->name, sizeof(adev->name), "%s",
>> device); 
>> 
> 
> I suggest replacing the parsing code with pci_parse_devaddr() (needs
> to be extended to support functions) so that all the checking and
> parsing is done in one place.

If use pci_parse_devaddr(), it needs to add domain section to assigning command, and add function section to pci_add/pci_del commands. What's more, pci_parse_devaddr() parses guest device bdf, there are some assumption, such as function is 0. But here parse host bdf. It's a little complex to combine them together.

Regards,
Weidong





--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Avi Kivity March 29, 2009, 2:35 p.m. UTC | #3
Han, Weidong wrote:
>> I suggest replacing the parsing code with pci_parse_devaddr() (needs
>> to be extended to support functions) so that all the checking and
>> parsing is done in one place.
>>     
>
> If use pci_parse_devaddr(), it needs to add domain section to assigning command, and add function section to pci_add/pci_del commands. What's more, pci_parse_devaddr() parses guest device bdf, there are some assumption, such as function is 0. But here parse host bdf. It's a little complex to combine them together.
>   

Right, but we end up with overall better code.
diff mbox

Patch

diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c
index cef7c8a..50a0d5c 100644
--- a/qemu/hw/device-assignment.c
+++ b/qemu/hw/device-assignment.c
@@ -1196,7 +1196,7 @@  out:
 AssignedDevInfo *add_assigned_device(const char *arg)
 {
     char *cp, *cp1;
-    char device[8];
+    char device[9];
     char dma[6];
     int r;
     AssignedDevInfo *adev;
@@ -1207,6 +1207,9 @@  AssignedDevInfo *add_assigned_device(const char *arg)
         return NULL;
     }
     r = get_param_value(device, sizeof(device), "host", arg);
+    /* b:d.f format: xx:yy.z */
+    if (r != 7)
+        goto bad;
     r = get_param_value(adev->name, sizeof(adev->name), "name", arg);
     if (!r)
 	snprintf(adev->name, sizeof(adev->name), "%s", device);