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

[Keir Fraser <Keir.Fraser@xxxxxxxxxxxx>]

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
> 



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