[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

RE: [Xen-devel] [VTD][PATCH] Don't allow assigning the samedevice twice without release the first assignment


  • To: "Keir Fraser" <Keir.Fraser@xxxxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxx>
  • From: "Han, Weidong" <weidong.han@xxxxxxxxx>
  • Date: Thu, 18 Oct 2007 23:02:00 +0800
  • Cc: "Kay, Allen M" <allen.m.kay@xxxxxxxxx>
  • Delivery-date: Thu, 18 Oct 2007 08:02:49 -0700
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>
  • Thread-index: AcgPPrZDfEJfzCOtSJyuGbCE3/cnAQAAjKktABVSMsAAClbYKwBaGWTgABAkgksACHz0QAAAw3BXAAKFRPA=
  • Thread-topic: [Xen-devel] [VTD][PATCH] Don't allow assigning the samedevice twice without release the first assignment

Have fixed it. New patch is attached. Thanks.

Randy

Keir Fraser wrote:
> Almost there, but you are misusing strtok_r(). The char** final
> argument has to be a pointer to the _same_ char* across a sequence of
> calls to strtok_r(). That is the whole point of it!
> 
> This means you cannot hide the char* inside first_bdf/next_bdf. You
> will have to add a char** final argument to first_bdf/next_bdf, and
> have the char* allocated in the caller's stack frame.
> 
> If you try a multi-device pci string with the current patch, you'll
> probably find you crash or something...
> 
> Try again. ;-)
> 
>  -- Keir
> 
> On 18/10/07 14:35, "Han, Weidong" <weidong.han@xxxxxxxxx> wrote:
> 
>> Keir, thanks for your valuable suggestion. I have changed the tools
>> part. As you said, PCI-string parsing is the small amount of code,
>> so I duplicate it in both python wrapper and ioemu. Attached patch
>> is new tools part patch. 
>> 
>> -- Weidong
>> 
>> Keir Fraser wrote:
>>> That's a lot better. I'll take the Xen portion but the tools
>>> changes need more work: 
>>> 
>>> The PCI-string parsing code cannot be placed in xc_private.[ch] and
>>> then exported outside the library. Also using strtok() is invalid as
>>> it is not thread-safe. You should use strtok_r() instead. I think
>>> you can get rid of pci_count() altogether and have
>>> first_bdf/next_bdf return a boolean whether they have reached the
>>> end of the string or not (strtok_r will return NULL when the last
>>> token has been parsed). Then the caller would use them something
>>>  like: for (done = first_bdf(); !done; done = next_bdf())
>>> 
>>> Given the small amount of code for first_bdf/next_bdf, and given
>>> that pci_count can be got rid of entirely, I would just duplicate
>>> that simple strtok code in both the python wrapper and ioemu. I
>>> wouldn't add that rather specific parsing code to libxc.
>>> 
>>> Please fix up and re-spin the tools part of the patch.
>>> 
>>>  -- Keir
>>> 
>>> On 18/10/07 02:51, "Han, Weidong" <weidong.han@xxxxxxxxx> wrote:
>>> 
>>>> Keir,
>>>> 
>>>> Resend the patch. This patch is implemented according to your
>>>> suggestion. Asssigns device in xend before starting ioemu, and add
>>>> the check on DOMCTL_assign_device hypercall. Thanks.
>>>> 
>>>> -- Weidong
>>>> 
>>>> Keir Fraser wrote:
>>>>> On 16/10/07 03:04, "Han, Weidong" <weidong.han@xxxxxxxxx> wrote:
>>>>> 
>>>>>>> The DOMCTL_assign_device should check whether the device is
>>>>>>> already assigned. This has the benefit that it can atomically
>>>>>>> check-and-allocate, under the domctl lock.
>>>>>>> 
>>>>>>> You'll have to work out how to propagate DOMCTL_assign_device
>>>>>>> failure to the user. Either you have to get the error out of
>>>>>>> ioemu, or perhaps you can assign the device in xend before
>>>>>>> starting ioemu.
>>>>>> 
>>>>>> Yes, adding the check on DOMCTL_assign_device is simple, but I
>>>>>> didn't find a good way to propagate DOMCTL_assign_device failure
>>>>>> to user. This patch adds the check in xend before starting ioemu.
>>>>>> In addtion, adding the check in Xend can prompt user on the
>>>>>> screen when the device has already been assigned. Thanks.
>>>>> 
>>>>> Well, that's too bad because I won't take the original patch. Why
>>>>> can't the device be assigned by xend?
>>>>> 
>>>>>  -- Keir
>>>> 
>>> 
>>> 
>>> 
>>> _______________________________________________
>>> Xen-devel mailing list
>>> Xen-devel@xxxxxxxxxxxxxxxxxxx
>>> http://lists.xensource.com/xen-devel

Attachment: tools-2.patch
Description: tools-2.patch

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.