I already checked in an appropriate fix following Christoph's comments.
-- Keir
On 30/1/08 16:24, "Pat Campbell" <plc@xxxxxxxxxx> wrote:
> Christoph Egger wrote:
>> On Wednesday 30 January 2008 15:14:24 Pat Campbell wrote:
>>
>>> More readable version
>>>
>>> Currently I am not able to mount or boot from an HVM CDROM when it is
>>> configured for 'tap:aio' instead of 'file'.
>>>
>>> disk=[ 'tap:aio:/var/lib/xen/images/sles10-sp2-fv/disk0,hda,w', '
>>> tap:aio:/home/iso/sles/SLES10.iso,hdc:cdrom,r', ]
>>>
>>> With the attached patch I am able to boot from the CDROM and or mount it.
>>>
>>> Patch changes xenstore.c:xenstore_process_event() to strip the tap subtype
>>> prefix from the image name.
>>>
>>> Please apply to xen-unstable tip.
>>>
>>> Signed-off-by: Pat Campbell <plc@xxxxxxxxxx>
>>>
>>
>> That looks much better.
>> See inline for comments.
>>
>>
>>> xen-blktab-subtype-strip.patch
>>> diff -r 1c826ea72a80 tools/ioemu/xenstore.c
>>> --- a/tools/ioemu/xenstore.c Wed Jan 23 15:42:52 2008 +0000
>>> +++ b/tools/ioemu/xenstore.c Wed Jan 30 07:06:12 2008 -0700
>>> @@ -418,7 +418,7 @@ void xenstore_record_dm_state(char *stat
>>>
>>> void xenstore_process_event(void *opaque)
>>> {
>>> - char **vec, *image = NULL;
>>> + char **vec, *offset, *bpath = NULL, *buf = NULL, *drv = NULL, *image =
>>>
>> NULL;
>>
>>> unsigned int len, num, hd_index;
>>>
>>> vec = xs_read_watch(xsh, &num);
>>> @@ -440,8 +440,28 @@ void xenstore_process_event(void *opaque
>>> goto out;
>>> hd_index = vec[XS_WATCH_TOKEN][2] - 'a';
>>> image = xs_read(xsh, XBT_NULL, vec[XS_WATCH_PATH], &len);
>>> - if (image == NULL || !strcmp(image, bs_table[hd_index]->filename))
>>> - goto out; /* gone or identical */
>>> + if (image == NULL)
>>> + goto out; /* gone */
>>> +
>>> + /* Strip off blktap sub-type prefix */
>>> + bpath = strdup(vec[XS_WATCH_PATH]);
>>> + if (bpath)
>>>
>>
>> I think, you mean if (!bpath) here.
>>
> You are right, missed that in my recode.
>>
>>> + goto out;
>>> + if ((offset = strrchr(bpath, '/')) != NULL)
>>> + *offset = '\0';
>>> + if (pasprintf(&buf, "%s/type", bpath) == -1)
>>> + goto out;
>>> + drv = xs_read(xsh, XBT_NULL, buf, &len);
>>> + if (drv) {
>>>
>>
>> I think, you mean "if (!drv) goto out;" here.
>>
> My thinking was, if we read the type we want to check for tap, otherwise
> we want to fall thru and do the normal image compare. Failure to read
> type is not be an error condition.
>
> It can be viewed as an error condition I suppose. Will change that to.
>>
>>> + if (!strcmp(drv, "tap")) {
>>> + offset = strchr(image, ':');
>>> + if (offset)
>>> + memmove(image, offset+1, strlen(offset+1)+1 );
>>> + }
>>> + }
>>> +
>>> + if (!strcmp(image, bs_table[hd_index]->filename))
>>> + goto out; /* identical */
>>>
>>> do_eject(0, vec[XS_WATCH_TOKEN]);
>>> bs_table[hd_index]->filename[0] = 0;
>>> @@ -456,6 +476,9 @@ void xenstore_process_event(void *opaque
>>> }
>>>
>>> out:
>>> + free(drv);
>>> + free(buf);
>>> + free(bpath);
>>> free(image);
>>> free(vec);
>>>
>>
>> That doesn't work. If strdup() fails, then drv and buf are NULL here.
>> Analagous counts for the other failures.
>> You need to check for != NULL before calling free().
>>
>>
> It appears this code style relies on free dealing with a NULL. Just
> trying to fit it :-)
>>> }
>>>
>>
>>
>>
> Thanks for taking the time.
>
> _______________________________________________
> 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
|
Home