Patchwork xen_disk: treat "aio" as "raw"

login
register
mail settings
Submitter Stefano Stabellini
Date June 24, 2011, 2:50 p.m.
Message ID <1308927034-10209-2-git-send-email-stefano.stabellini@eu.citrix.com>
Download mbox | patch
Permalink /patch/101815/
State New
Headers show

Comments

Stefano Stabellini - June 24, 2011, 2:50 p.m.
From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Sometimes the toolstack uses "aio" without an additional format
identifier, in such cases use "raw".

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 hw/xen_disk.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Alexander Graf - June 30, 2011, 11:47 a.m.
On 06/24/2011 04:50 PM, stefano.stabellini@eu.citrix.com wrote:
> From: Stefano Stabellini<stefano.stabellini@eu.citrix.com>
>
> Sometimes the toolstack uses "aio" without an additional format
> identifier, in such cases use "raw".

Shouldn't this rather be a patch to the toolstack then? We do automatic 
file format recognition as default. I find magical "aio equals raw" 
subtleties rather unintuitive.


> Signed-off-by: Stefano Stabellini<stefano.stabellini@eu.citrix.com>
> ---
>   hw/xen_disk.c |    2 ++
>   1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/hw/xen_disk.c b/hw/xen_disk.c
> index 801da58..745127d 100644
> --- a/hw/xen_disk.c
> +++ b/hw/xen_disk.c
> @@ -633,6 +633,8 @@ static int blk_init(struct XenDevice *xendev)
>               blkdev->filename  = blkdev->params;
>           }
>       }
> +    if (!strcmp("aio", blkdev->fileproto))
> +        blkdev->fileproto = "raw";

Braces

>       if (blkdev->mode == NULL) {
>           blkdev->mode = xenstore_read_be_str(&blkdev->xendev, "mode");
>       }


Alex
Alexander Graf - June 30, 2011, 12:45 p.m.
On 06/30/2011 02:47 PM, Kevin Wolf wrote:
> Am 30.06.2011 13:47, schrieb Alexander Graf:
>> On 06/24/2011 04:50 PM, stefano.stabellini@eu.citrix.com wrote:
>>> From: Stefano Stabellini<stefano.stabellini@eu.citrix.com>
>>>
>>> Sometimes the toolstack uses "aio" without an additional format
>>> identifier, in such cases use "raw".
>> Shouldn't this rather be a patch to the toolstack then? We do automatic
>> file format recognition as default. I find magical "aio equals raw"
>> subtleties rather unintuitive.
> The asynchronous raw blktap driver is (was?) called "aio", so I guess
> it's to stay compatible with that. Has always been this way in Xen (and
> has always been confusing). As long as it stays in xen_disk.c, I don't
> really mind...

Is there a realistic chance of changing the toolstack here? I'd really 
prefer not to drag this legacy around :).


Alex
Kevin Wolf - June 30, 2011, 12:47 p.m.
Am 30.06.2011 13:47, schrieb Alexander Graf:
> On 06/24/2011 04:50 PM, stefano.stabellini@eu.citrix.com wrote:
>> From: Stefano Stabellini<stefano.stabellini@eu.citrix.com>
>>
>> Sometimes the toolstack uses "aio" without an additional format
>> identifier, in such cases use "raw".
> 
> Shouldn't this rather be a patch to the toolstack then? We do automatic 
> file format recognition as default. I find magical "aio equals raw" 
> subtleties rather unintuitive.

The asynchronous raw blktap driver is (was?) called "aio", so I guess
it's to stay compatible with that. Has always been this way in Xen (and
has always been confusing). As long as it stays in xen_disk.c, I don't
really mind...

Kevin
Stefano Stabellini - June 30, 2011, 2:20 p.m.
On Thu, 30 Jun 2011, Alexander Graf wrote:
> On 06/30/2011 02:47 PM, Kevin Wolf wrote:
> > Am 30.06.2011 13:47, schrieb Alexander Graf:
> >> On 06/24/2011 04:50 PM, stefano.stabellini@eu.citrix.com wrote:
> >>> From: Stefano Stabellini<stefano.stabellini@eu.citrix.com>
> >>>
> >>> Sometimes the toolstack uses "aio" without an additional format
> >>> identifier, in such cases use "raw".
> >> Shouldn't this rather be a patch to the toolstack then? We do automatic
> >> file format recognition as default. I find magical "aio equals raw"
> >> subtleties rather unintuitive.
> > The asynchronous raw blktap driver is (was?) called "aio", so I guess
> > it's to stay compatible with that. Has always been this way in Xen (and
> > has always been confusing). As long as it stays in xen_disk.c, I don't
> > really mind...
> 
> Is there a realistic chance of changing the toolstack here? I'd really 
> prefer not to drag this legacy around :).

We could change the toolstack but it would mean introducing an
incompatibility between the qdisk backend and the blktap backend.
Considering that qdisk was written to replace blktap in the first place
I don't think is such a good idea. Besides all xen toolstacks have
always written "aio" for "raw" even before qdisk came along...
Alexander Graf - June 30, 2011, 2:32 p.m.
On 06/30/2011 04:20 PM, Stefano Stabellini wrote:
> On Thu, 30 Jun 2011, Alexander Graf wrote:
>> On 06/30/2011 02:47 PM, Kevin Wolf wrote:
>>> Am 30.06.2011 13:47, schrieb Alexander Graf:
>>>> On 06/24/2011 04:50 PM, stefano.stabellini@eu.citrix.com wrote:
>>>>> From: Stefano Stabellini<stefano.stabellini@eu.citrix.com>
>>>>>
>>>>> Sometimes the toolstack uses "aio" without an additional format
>>>>> identifier, in such cases use "raw".
>>>> Shouldn't this rather be a patch to the toolstack then? We do automatic
>>>> file format recognition as default. I find magical "aio equals raw"
>>>> subtleties rather unintuitive.
>>> The asynchronous raw blktap driver is (was?) called "aio", so I guess
>>> it's to stay compatible with that. Has always been this way in Xen (and
>>> has always been confusing). As long as it stays in xen_disk.c, I don't
>>> really mind...
>> Is there a realistic chance of changing the toolstack here? I'd really
>> prefer not to drag this legacy around :).
> We could change the toolstack but it would mean introducing an
> incompatibility between the qdisk backend and the blktap backend.
> Considering that qdisk was written to replace blktap in the first place
> I don't think is such a good idea. Besides all xen toolstacks have
> always written "aio" for "raw" even before qdisk came along...

Hrm. Ok. Please resend with braces then :).


Alex
Stefano Stabellini - June 30, 2011, 2:39 p.m.
On Thu, 30 Jun 2011, Alexander Graf wrote:
> On 06/30/2011 04:20 PM, Stefano Stabellini wrote:
> > On Thu, 30 Jun 2011, Alexander Graf wrote:
> >> On 06/30/2011 02:47 PM, Kevin Wolf wrote:
> >>> Am 30.06.2011 13:47, schrieb Alexander Graf:
> >>>> On 06/24/2011 04:50 PM, stefano.stabellini@eu.citrix.com wrote:
> >>>>> From: Stefano Stabellini<stefano.stabellini@eu.citrix.com>
> >>>>>
> >>>>> Sometimes the toolstack uses "aio" without an additional format
> >>>>> identifier, in such cases use "raw".
> >>>> Shouldn't this rather be a patch to the toolstack then? We do automatic
> >>>> file format recognition as default. I find magical "aio equals raw"
> >>>> subtleties rather unintuitive.
> >>> The asynchronous raw blktap driver is (was?) called "aio", so I guess
> >>> it's to stay compatible with that. Has always been this way in Xen (and
> >>> has always been confusing). As long as it stays in xen_disk.c, I don't
> >>> really mind...
> >> Is there a realistic chance of changing the toolstack here? I'd really
> >> prefer not to drag this legacy around :).
> > We could change the toolstack but it would mean introducing an
> > incompatibility between the qdisk backend and the blktap backend.
> > Considering that qdisk was written to replace blktap in the first place
> > I don't think is such a good idea. Besides all xen toolstacks have
> > always written "aio" for "raw" even before qdisk came along...
> 
> Hrm. Ok. Please resend with braces then :).

I'll do :)

Patch

diff --git a/hw/xen_disk.c b/hw/xen_disk.c
index 801da58..745127d 100644
--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -633,6 +633,8 @@  static int blk_init(struct XenDevice *xendev)
             blkdev->filename  = blkdev->params;
         }
     }
+    if (!strcmp("aio", blkdev->fileproto))
+        blkdev->fileproto = "raw";
     if (blkdev->mode == NULL) {
         blkdev->mode = xenstore_read_be_str(&blkdev->xendev, "mode");
     }