Xen devel - Aug 2013 - [PATCH] libxl: prefer qdisk over blktap when choosing disk backend

There are some disk formats commonly supported by both qdisk and blktap.
As qdisk is better supported and blktap is unmaintained, we choose qdisk
over blktap whenever possible.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxl/libxl_device.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
index ea845b7..16a92a4 100644
--- a/tools/libxl/libxl_device.c
+++ b/tools/libxl/libxl_device.c
@@ -288,8 +288,8 @@ int libxl__device_disk_set_backend(libxl__gc *gc,
libxl_device_disk *disk) {
     } else {
         ok             disk_try_backend(&a, LIBXL_DISK_BACKEND_PHY) ?:
-            disk_try_backend(&a, LIBXL_DISK_BACKEND_TAP) ?:
-            disk_try_backend(&a, LIBXL_DISK_BACKEND_QDISK);
+            disk_try_backend(&a, LIBXL_DISK_BACKEND_QDISK) ?:
+            disk_try_backend(&a, LIBXL_DISK_BACKEND_TAP);
         if (ok)
             LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Disk vdev=%s, using backend
%s",
                        disk->vdev,
-- 
1.7.10.4

CCing IanJ and Stefano (forgot to do this when sending the patch)...

On Tue, Aug 27, 2013 at 03:22:43PM +0100, Wei Liu wrote:
> There are some disk formats commonly supported by both qdisk and blktap.
> As qdisk is better supported and blktap is unmaintained, we choose qdisk
> over blktap whenever possible.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
>  tools/libxl/libxl_device.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index ea845b7..16a92a4 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -288,8 +288,8 @@ int libxl__device_disk_set_backend(libxl__gc *gc,
libxl_device_disk *disk) {
>      } else {
>          ok>              disk_try_backend(&a,
LIBXL_DISK_BACKEND_PHY) ?:
> -            disk_try_backend(&a, LIBXL_DISK_BACKEND_TAP) ?:
> -            disk_try_backend(&a, LIBXL_DISK_BACKEND_QDISK);
> +            disk_try_backend(&a, LIBXL_DISK_BACKEND_QDISK) ?:
> +            disk_try_backend(&a, LIBXL_DISK_BACKEND_TAP);
>          if (ok)
>              LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Disk vdev=%s, using
backend %s",
>                         disk->vdev,
> -- 
> 1.7.10.4

Wei Liu writes ("Re: [PATCH] libxl: prefer qdisk over blktap when choosing
disk backend"):
> On Tue, Aug 27, 2013 at 03:22:43PM +0100, Wei Liu wrote:
> > There are some disk formats commonly supported by both qdisk and
blktap.
> > As qdisk is better supported and blktap is unmaintained, we choose
qdisk
> > over blktap whenever possible.
The reason for this ordering was that most kernels nowdays don''t have
blktap.  So you don''t get blktap on new systems, only on old ones.
That''s good for stability I think.

Does that make any kind of sense ?

Ian.

On Wed, Aug 28, 2013 at 11:35:50AM +0100, Ian Jackson
wrote:
> Wei Liu writes ("Re: [PATCH] libxl: prefer qdisk over blktap when
choosing disk backend"):
> > On Tue, Aug 27, 2013 at 03:22:43PM +0100, Wei Liu wrote:
> > > There are some disk formats commonly supported by both qdisk and
blktap.
> > > As qdisk is better supported and blktap is unmaintained, we
choose qdisk
> > > over blktap whenever possible.
> 
> The reason for this ordering was that most kernels nowdays don''t
have
> blktap.  So you don''t get blktap on new systems, only on old ones.
> That''s good for stability I think.
> 
> Does that make any kind of sense ?
> 
What about installing newer Xen on old systems which have blktap? In
that case users can have better supported backend.

Wei.
> Ian.

On Wed, Aug 28, 2013 at 11:35 AM, Ian Jackson <Ian.Jackson@eu.citrix.com>
wrote:
> Wei Liu writes ("Re: [PATCH] libxl: prefer qdisk over blktap when
choosing disk backend"):
>> On Tue, Aug 27, 2013 at 03:22:43PM +0100, Wei Liu wrote:
>> > There are some disk formats commonly supported by both qdisk and
blktap.
>> > As qdisk is better supported and blktap is unmaintained, we choose
qdisk
>> > over blktap whenever possible.
>
> The reason for this ordering was that most kernels nowdays don''t
have
> blktap.  So you don''t get blktap on new systems, only on old ones.
> That''s good for stability I think.
>
> Does that make any kind of sense ?
I don''t think the original reasoning makes sense.  On systems without
blktap, the order of blktap / qdisk doesn''t matter in the slightest.
The order only matters on old systems, with blktap; and there it''s
better to use qdisk if possible, because it''s maintained and blktap
isn''t.

 -George

Il 28/08/2013 12:59, George Dunlap ha scritto:
> On Wed, Aug 28, 2013 at 11:35 AM, Ian Jackson
<Ian.Jackson@eu.citrix.com> wrote:
>> Wei Liu writes ("Re: [PATCH] libxl: prefer qdisk over blktap when
choosing disk backend"):
>>> On Tue, Aug 27, 2013 at 03:22:43PM +0100, Wei Liu wrote:
>>>> There are some disk formats commonly supported by both qdisk
and blktap.
>>>> As qdisk is better supported and blktap is unmaintained, we
choose qdisk
>>>> over blktap whenever possible.
>> The reason for this ordering was that most kernels nowdays
don''t have
>> blktap.  So you don''t get blktap on new systems, only on old
ones.
>> That''s good for stability I think.
>>
>> Does that make any kind of sense ?
> I don''t think the original reasoning makes sense.  On systems
without
> blktap, the order of blktap / qdisk doesn''t matter in the
slightest.
> The order only matters on old systems, with blktap; and there it''s
> better to use qdisk if possible, because it''s maintained and
blktap
> isn''t.
>
>   -George
I think is good prefer qdisk also for significant performance increase 
in comparison with blktap2.
On attachment one benchmark about qdisk and blktap2 of previous month on 
windows 7 domU.
I used only qdisk on my test system for one month without found bugs for 
now.
Blktap is used only with xen, qdisk also with other softwares using 
qemu, is more supported/tested.
I think is better point to upstream software and/or software used also 
by other opensource project for have optimize the development / testing 
and then have a final solution as best quality, performance and reliability.
A good example are moved to upstream kernel and qemu (qemu still not 
enough in my opinion, I think that major of developer/users are wasting 
too much time on traditional while the upstream is now little used and 
tested unfortunately).
The only question that comes about this patch is about "the qemu 
traditional case", although I think it should be abandoned as soon as 
possible to point all to the upstream qemu.

Sorry for my bad english.
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

On Wed, Aug 28, 2013 at 01:53:31PM +0200, Fabio Fantoni wrote:
[...]
> The only question that comes about this patch is about "the qemu
> traditional case", although I think it should be abandoned as soon
I''m not sure I understand your question here, but...
> as possible to point all to the upstream qemu.
opting out trad-qemu is out of scope of this patch IMHO.

Wei.
> 
> Sorry for my bad english.
> 
> >
> >_______________________________________________
> >Xen-devel mailing list
> >Xen-devel@lists.xen.org
> >http://lists.xen.org/xen-devel
>

Fabio Fantoni writes ("Re: [Xen-devel] [PATCH] libxl: prefer qdisk over
blktap when choosing disk backend"):
> I think is good prefer qdisk also for significant performance increase 
> in comparison with blktap2.
Thanks, that''s useful information.  That, and what George said, have
convinced me this is the right change.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Il 28/08/2013 15:04, Ian Jackson ha scritto:
> Fabio Fantoni writes ("Re: [Xen-devel] [PATCH] libxl: prefer qdisk
over blktap when choosing disk backend"):
>> I think is good prefer qdisk also for significant performance increase
>> in comparison with blktap2.
> Thanks, that''s useful information.  That, and what George said,
have
> convinced me this is the right change.
>
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
What about qemu traditional with this patch?
I haven''t tested qdisk with qemu traditional but unfortunately 
qemu-trad. is still widely used and therefore you have to be sure that 
does not cause problems.
I asked about because devices parts seem the same with both qemu but I 
not sure about it.

On Wed, Aug 28, 2013 at 03:16:23PM +0200, Fabio Fantoni
wrote:
> Il 28/08/2013 15:04, Ian Jackson ha scritto:
> >Fabio Fantoni writes ("Re: [Xen-devel] [PATCH] libxl: prefer qdisk
over blktap when choosing disk backend"):
> >>I think is good prefer qdisk also for significant performance
increase
> >>in comparison with blktap2.
> >Thanks, that''s useful information.  That, and what George
said, have
> >convinced me this is the right change.
> >
> >Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> What about qemu traditional with this patch?
> I haven''t tested qdisk with qemu traditional but unfortunately
> qemu-trad. is still widely used and therefore you have to be sure
> that does not cause problems.
Ah, now I get your question.
> I asked about because devices parts seem the same with both qemu but
> I not sure about it.
We''ve already switched to qemu-upstream in 4.3. And this patch is not
backport material so old system would just work fine IMHO.

The only risk of breakage is: users have device_model_version set to
qemu-trad and run blktap kernel with Xen pre-4.3, then upgrade to Xen
post-4.3 (with this patch). I''ve tested that, and qemu-trad runs fine
for me -- at least it boots and dd works well.

Wei.

On Wed, Aug 28, 2013 at 02:35:56PM +0100, Wei Liu wrote:
> On Wed, Aug 28, 2013 at 03:16:23PM +0200, Fabio Fantoni wrote:
> > Il 28/08/2013 15:04, Ian Jackson ha scritto:
> > >Fabio Fantoni writes ("Re: [Xen-devel] [PATCH] libxl: prefer
qdisk over blktap when choosing disk backend"):
> > >>I think is good prefer qdisk also for significant performance
increase
> > >>in comparison with blktap2.
> > >Thanks, that''s useful information.  That, and what George
said, have
> > >convinced me this is the right change.
> > >
> > >Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> > 
> > What about qemu traditional with this patch?
> > I haven''t tested qdisk with qemu traditional but
unfortunately
> > qemu-trad. is still widely used and therefore you have to be sure
> > that does not cause problems.
> 
> Ah, now I get your question.
> 
> > I asked about because devices parts seem the same with both qemu but
> > I not sure about it.
> 
> We''ve already switched to qemu-upstream in 4.3. And this patch is
not
> backport material so old system would just work fine IMHO.
> 
> The only risk of breakage is: users have device_model_version set to
> qemu-trad and run blktap kernel with Xen pre-4.3, then upgrade to Xen
> post-4.3 (with this patch). I''ve tested that, and qemu-trad runs
fine
> for me -- at least it boots and dd works well.
> 
I didn''t have a VHD image (format commonly supported by blktap and
qemu)
at hand so the test was not complete. But qemu-trad and qemu-xen are
both maintained so even if it breaks we are able to fix them -- which is
main point of this patch, to let users have better supported backend.

Wei.
> Wei.

On 28/08/13 15:41, Wei Liu wrote:
> On Wed, Aug 28, 2013 at 02:35:56PM +0100, Wei Liu wrote:
>> On Wed, Aug 28, 2013 at 03:16:23PM +0200, Fabio Fantoni wrote:
>>> Il 28/08/2013 15:04, Ian Jackson ha scritto:
>>>> Fabio Fantoni writes ("Re: [Xen-devel] [PATCH] libxl:
prefer qdisk over blktap when choosing disk backend"):
>>>>> I think is good prefer qdisk also for significant
performance increase
>>>>> in comparison with blktap2.
>>>> Thanks, that''s useful information.  That, and what
George said, have
>>>> convinced me this is the right change.
>>>>
>>>> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
>>>
>>> What about qemu traditional with this patch?
>>> I haven''t tested qdisk with qemu traditional but
unfortunately
>>> qemu-trad. is still widely used and therefore you have to be sure
>>> that does not cause problems.
>>
>> Ah, now I get your question.
>>
>>> I asked about because devices parts seem the same with both qemu
but
>>> I not sure about it.
>>
>> We''ve already switched to qemu-upstream in 4.3. And this patch
is not
>> backport material so old system would just work fine IMHO.
>>
>> The only risk of breakage is: users have device_model_version set to
>> qemu-trad and run blktap kernel with Xen pre-4.3, then upgrade to Xen
>> post-4.3 (with this patch). I''ve tested that, and qemu-trad
runs fine
>> for me -- at least it boots and dd works well.
>>
> 
> I didn''t have a VHD image (format commonly supported by blktap and
qemu)
> at hand so the test was not complete. But qemu-trad and qemu-xen are
> both maintained so even if it breaks we are able to fix them -- which is
> main point of this patch, to let users have better supported backend.
I might be completely wrong, but wasn''t there a problem when using
blktap VHD images with Qemu (ie VHD images created with blktap weren''t
copatible with VHD Qemu implementation)?

On Wed, Aug 28, 2013 at 05:04:07PM +0200, Roger Pau Monné
wrote:
> On 28/08/13 15:41, Wei Liu wrote:
> > On Wed, Aug 28, 2013 at 02:35:56PM +0100, Wei Liu wrote:
> >> On Wed, Aug 28, 2013 at 03:16:23PM +0200, Fabio Fantoni wrote:
> >>> Il 28/08/2013 15:04, Ian Jackson ha scritto:
> >>>> Fabio Fantoni writes ("Re: [Xen-devel] [PATCH] libxl:
prefer qdisk over blktap when choosing disk backend"):
> >>>>> I think is good prefer qdisk also for significant
performance increase
> >>>>> in comparison with blktap2.
> >>>> Thanks, that''s useful information.  That, and
what George said, have
> >>>> convinced me this is the right change.
> >>>>
> >>>> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> >>>
> >>> What about qemu traditional with this patch?
> >>> I haven''t tested qdisk with qemu traditional but
unfortunately
> >>> qemu-trad. is still widely used and therefore you have to be
sure
> >>> that does not cause problems.
> >>
> >> Ah, now I get your question.
> >>
> >>> I asked about because devices parts seem the same with both
qemu but
> >>> I not sure about it.
> >>
> >> We''ve already switched to qemu-upstream in 4.3. And this
patch is not
> >> backport material so old system would just work fine IMHO.
> >>
> >> The only risk of breakage is: users have device_model_version set
to
> >> qemu-trad and run blktap kernel with Xen pre-4.3, then upgrade to
Xen
> >> post-4.3 (with this patch). I''ve tested that, and
qemu-trad runs fine
> >> for me -- at least it boots and dd works well.
> >>
> > 
> > I didn''t have a VHD image (format commonly supported by
blktap and qemu)
> > at hand so the test was not complete. But qemu-trad and qemu-xen are
> > both maintained so even if it breaks we are able to fix them -- which
is
> > main point of this patch, to let users have better supported backend.
> 
> I might be completely wrong, but wasn''t there a problem when using
> blktap VHD images with Qemu (ie VHD images created with blktap
weren''t
> copatible with VHD Qemu implementation)?
Do you mean that timestamp bug in libvhd [0]? Oh that''s not fixed in
OSS
Xen.

Ian, do you think it is necessary to have [1] ported to OSS Xen?

Then there''s all the old images needed to be converted. However this is
a necessary step sooner or later if users migrate these images to newer
Xen + newer kernel which don''t have blktap anymore.

Wei.

[0] http://lists.xen.org/archives/html/xen-devel/2013-02/msg01326.html
[1]
https://github.com/xapi-project/blktap/commit/a79ac2c05f97c2384bbf981419f329f184dc646a

On Wed, 2013-08-28 at 14:04 +0100, Ian Jackson wrote:
> Fabio Fantoni writes ("Re: [Xen-devel] [PATCH] libxl: prefer qdisk
over blktap when choosing disk backend"):
> > I think is good prefer qdisk also for significant performance increase
> > in comparison with blktap2.
> 
> Thanks, that''s useful information.  That, and what George said,
have
> convinced me this is the right change.
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
applied.