Patchwork [2/2] Added target to build libvdisk

login
register
mail settings
Submitter Saggi Mizrahi
Date Aug. 22, 2011, 5:06 p.m.
Message ID <1314032798-21423-2-git-send-email-smizrahi@redhat.com>
Download mbox | patch
Permalink /patch/110959/
State New
Headers show

Comments

Saggi Mizrahi - Aug. 22, 2011, 5:06 p.m.
libvdisk is a library that packages qemu's handling of disk images. This
allows for other programs to link to it and get access to qemu image
file abstractions.

To use install the lib and #include <vdisk/block.h>
all the bdrv_* functions work as expected.

Signed-off-by: Saggi Mizrahi <smizrahi@redhat.com>
---
 .gitignore    |    4 ++--
 Makefile.objs |    9 +++++++++
 2 files changed, 11 insertions(+), 2 deletions(-)
Blue Swirl - Aug. 22, 2011, 6:50 p.m.
On Mon, Aug 22, 2011 at 5:06 PM, Saggi Mizrahi <smizrahi@redhat.com> wrote:
> libvdisk is a library that packages qemu's handling of disk images. This
> allows for other programs to link to it and get access to qemu image
> file abstractions.
>
> To use install the lib and #include <vdisk/block.h>
> all the bdrv_* functions work as expected.

So far, exporting QEMU internals as a library has not been accepted,
because we don't want the burden of maintaining the stability of any
APIs or ABIs.

> Signed-off-by: Saggi Mizrahi <smizrahi@redhat.com>
> ---
>  .gitignore    |    4 ++--
>  Makefile.objs |    9 +++++++++
>  2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/.gitignore b/.gitignore
> index 59c343c..a389059 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -1,5 +1,4 @@
> -config-devices.*
> -config-all-devices.*
> +config-devices.* config-all-devices.*

Bug.

>  config-host.*
>  config-target.*
>  trace.h
> @@ -15,6 +14,7 @@ libdis*
>  libhw32
>  libhw64
>  libuser
> +libvdisk
>  qapi-generated
>  qemu-doc.html
>  qemu-tech.html
> diff --git a/Makefile.objs b/Makefile.objs
> index 432b619..291f194 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -171,6 +171,15 @@ common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o
>  common-obj-$(CONFIG_XEN_BACKEND) += xen_console.o xenfb.o xen_disk.o xen_nic.o
>
>  ######################################################################
> +# libvdisk
> +
> +vdisk-obj-y = $(block-obj-y)
> +
> +vdisk-obj-y += qemu-tool.o qemu-error.o
> +vdisk-obj-y += $(oslib-obj-y) $(trace-obj-y) $(block-obj-y)
> +vdisk-obj-y += $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o
> +
> +######################################################################
>  # libuser
>
>  user-obj-y =
> --
> 1.7.6
>
>
>
Anthony Liguori - Aug. 22, 2011, 7:29 p.m.
On 08/22/2011 12:06 PM, Saggi Mizrahi wrote:
> libvdisk is a library that packages qemu's handling of disk images. This
> allows for other programs to link to it and get access to qemu image
> file abstractions.
>
> To use install the lib and #include<vdisk/block.h>
> all the bdrv_* functions work as expected.
>
> Signed-off-by: Saggi Mizrahi<smizrahi@redhat.com>

It's a good idea in principle but the approach is far too naive.

The block layer needs a good bit of modularization first.  Test cases 
need to be written, and most importantly, using the library shouldn't 
require writing a bunch of dummy functions.

Regards,

Anthony Liguori

> ---
>   .gitignore    |    4 ++--
>   Makefile.objs |    9 +++++++++
>   2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/.gitignore b/.gitignore
> index 59c343c..a389059 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -1,5 +1,4 @@
> -config-devices.*
> -config-all-devices.*
> +config-devices.* config-all-devices.*
>   config-host.*
>   config-target.*
>   trace.h
> @@ -15,6 +14,7 @@ libdis*
>   libhw32
>   libhw64
>   libuser
> +libvdisk
>   qapi-generated
>   qemu-doc.html
>   qemu-tech.html
> diff --git a/Makefile.objs b/Makefile.objs
> index 432b619..291f194 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -171,6 +171,15 @@ common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o
>   common-obj-$(CONFIG_XEN_BACKEND) += xen_console.o xenfb.o xen_disk.o xen_nic.o
>
>   ######################################################################
> +# libvdisk
> +
> +vdisk-obj-y = $(block-obj-y)
> +
> +vdisk-obj-y += qemu-tool.o qemu-error.o
> +vdisk-obj-y += $(oslib-obj-y) $(trace-obj-y) $(block-obj-y)
> +vdisk-obj-y += $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o
> +
> +######################################################################
>   # libuser
>
>   user-obj-y =
Daniel P. Berrange - Aug. 23, 2011, 4:12 p.m.
On Mon, Aug 22, 2011 at 02:29:00PM -0500, Anthony Liguori wrote:
> On 08/22/2011 12:06 PM, Saggi Mizrahi wrote:
> >libvdisk is a library that packages qemu's handling of disk images. This
> >allows for other programs to link to it and get access to qemu image
> >file abstractions.
> >
> >To use install the lib and #include<vdisk/block.h>
> >all the bdrv_* functions work as expected.
> >
> >Signed-off-by: Saggi Mizrahi<smizrahi@redhat.com>
> 
> It's a good idea in principle but the approach is far too naive.
> 
> The block layer needs a good bit of modularization first.  Test
> cases need to be written, and most importantly, using the library
> shouldn't require writing a bunch of dummy functions.

There are also licensing practicalities which can cause us
some trouble/pain. For example:

> >  ######################################################################
> >+# libvdisk
> >+
> >+vdisk-obj-y = $(block-obj-y)
> >+
> >+vdisk-obj-y += qemu-tool.o qemu-error.o
> >+vdisk-obj-y += $(oslib-obj-y) $(trace-obj-y) $(block-obj-y)
> >+vdisk-obj-y += $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o

$(block-obj-y) pulls in  'aio.o' which is built from aio.c which
is licensed  "GPLv2 only". So even those many files are BSD
licenses, the combined work will be GPLv2-only. Unfortunately ending
up with a libqemublock.so which is GPLv2-only is as good as useless
for libs/apps since it is incompatible with both LGPLv2(+) and GPLv3.

Now in this case aio.c is labelled as Copyright IBM / Anthony,
so IBM could likely resolve this licensing to be more widely
compatible. This could^H^Hwould become a non-trivial task if we
need to look at many files & then also any patches accepted to
those files from 3rd parties over the years :-(

Regards,
Daniel
Anthony Liguori - Aug. 23, 2011, 4:14 p.m.
On 08/23/2011 11:12 AM, Daniel P. Berrange wrote:
> On Mon, Aug 22, 2011 at 02:29:00PM -0500, Anthony Liguori wrote:
>> On 08/22/2011 12:06 PM, Saggi Mizrahi wrote:
>>> libvdisk is a library that packages qemu's handling of disk images. This
>>> allows for other programs to link to it and get access to qemu image
>>> file abstractions.
>>>
>>> To use install the lib and #include<vdisk/block.h>
>>> all the bdrv_* functions work as expected.
>>>
>>> Signed-off-by: Saggi Mizrahi<smizrahi@redhat.com>
>>
>> It's a good idea in principle but the approach is far too naive.
>>
>> The block layer needs a good bit of modularization first.  Test
>> cases need to be written, and most importantly, using the library
>> shouldn't require writing a bunch of dummy functions.
>
> There are also licensing practicalities which can cause us
> some trouble/pain. For example:
>
>>>   ######################################################################
>>> +# libvdisk
>>> +
>>> +vdisk-obj-y = $(block-obj-y)
>>> +
>>> +vdisk-obj-y += qemu-tool.o qemu-error.o
>>> +vdisk-obj-y += $(oslib-obj-y) $(trace-obj-y) $(block-obj-y)
>>> +vdisk-obj-y += $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o
>
> $(block-obj-y) pulls in  'aio.o' which is built from aio.c which
> is licensed  "GPLv2 only". So even those many files are BSD
> licenses, the combined work will be GPLv2-only. Unfortunately ending
> up with a libqemublock.so which is GPLv2-only is as good as useless
> for libs/apps since it is incompatible with both LGPLv2(+) and GPLv3.
>
> Now in this case aio.c is labelled as Copyright IBM / Anthony,
> so IBM could likely resolve this licensing to be more widely
> compatible. This could^H^Hwould become a non-trivial task if we
> need to look at many files&  then also any patches accepted to
> those files from 3rd parties over the years :-(

If there was a block driver library, I would expect it to be GPL, not LGPL.

Regards,

Anthony Liguori

>
> Regards,
> Daniel
Daniel P. Berrange - Aug. 23, 2011, 4:18 p.m.
On Tue, Aug 23, 2011 at 11:14:20AM -0500, Anthony Liguori wrote:
> On 08/23/2011 11:12 AM, Daniel P. Berrange wrote:
> >$(block-obj-y) pulls in  'aio.o' which is built from aio.c which
> >is licensed  "GPLv2 only". So even those many files are BSD
> >licenses, the combined work will be GPLv2-only. Unfortunately ending
> >up with a libqemublock.so which is GPLv2-only is as good as useless
> >for libs/apps since it is incompatible with both LGPLv2(+) and GPLv3.
> >
> >Now in this case aio.c is labelled as Copyright IBM / Anthony,
> >so IBM could likely resolve this licensing to be more widely
> >compatible. This could^H^Hwould become a non-trivial task if we
> >need to look at many files&  then also any patches accepted to
> >those files from 3rd parties over the years :-(
> 
> If there was a block driver library, I would expect it to be GPL, not LGPL.

This would prevent us from using it in libvirt, unless we wrote a
helper program which we spawned anytime we wanted to use some
functionality library :-(

Regards,
Daniel
Anthony Liguori - Aug. 23, 2011, 4:21 p.m.
On 08/23/2011 11:18 AM, Daniel P. Berrange wrote:
> On Tue, Aug 23, 2011 at 11:14:20AM -0500, Anthony Liguori wrote:
>> On 08/23/2011 11:12 AM, Daniel P. Berrange wrote:
>>> $(block-obj-y) pulls in  'aio.o' which is built from aio.c which
>>> is licensed  "GPLv2 only". So even those many files are BSD
>>> licenses, the combined work will be GPLv2-only. Unfortunately ending
>>> up with a libqemublock.so which is GPLv2-only is as good as useless
>>> for libs/apps since it is incompatible with both LGPLv2(+) and GPLv3.
>>>
>>> Now in this case aio.c is labelled as Copyright IBM / Anthony,
>>> so IBM could likely resolve this licensing to be more widely
>>> compatible. This could^H^Hwould become a non-trivial task if we
>>> need to look at many files&   then also any patches accepted to
>>> those files from 3rd parties over the years :-(
>>
>> If there was a block driver library, I would expect it to be GPL, not LGPL.
>
> This would prevent us from using it in libvirt, unless we wrote a
> helper program which we spawned anytime we wanted to use some
> functionality library :-(

libvirtd is GPL, no?

But QEMU is GPL.  Libraries derived from QEMU will also be GPL.

Regards,

Anthony Liguori

>
> Regards,
> Daniel
Saggi Mizrahi - Aug. 24, 2011, 11:32 a.m.
On Tue 23 Aug 2011 07:21:56 PM IDT, Anthony Liguori wrote:
> On 08/23/2011 11:18 AM, Daniel P. Berrange wrote:
>> On Tue, Aug 23, 2011 at 11:14:20AM -0500, Anthony Liguori wrote:
>>> On 08/23/2011 11:12 AM, Daniel P. Berrange wrote:
>>>> $(block-obj-y) pulls in 'aio.o' which is built from aio.c which
>>>> is licensed "GPLv2 only". So even those many files are BSD
>>>> licenses, the combined work will be GPLv2-only. Unfortunately ending
>>>> up with a libqemublock.so which is GPLv2-only is as good as useless
>>>> for libs/apps since it is incompatible with both LGPLv2(+) and GPLv3.
>>>>
>>>> Now in this case aio.c is labelled as Copyright IBM / Anthony,
>>>> so IBM could likely resolve this licensing to be more widely
>>>> compatible. This could^H^Hwould become a non-trivial task if we
>>>> need to look at many files& then also any patches accepted to
>>>> those files from 3rd parties over the years :-(
>>>
>>> If there was a block driver library, I would expect it to be GPL, not 
>>> LGPL.
>>
>> This would prevent us from using it in libvirt, unless we wrote a
>> helper program which we spawned anytime we wanted to use some
>> functionality library :-(
> 
> libvirtd is GPL, no?
> 
> But QEMU is GPL. Libraries derived from QEMU will also be GPL.
> 
> Regards,
> 
> Anthony Liguori
> 
>>
>> Regards,
>> Daniel

OK, I admit it was a pretty naive solution. But I always try to do the 
simplest solution first.
The license issues can be solved later. (Having it as GPL later 
changing to LGPL if we can).

As for the API I can create start a specialized API for the lib that 
uses the internal API.
I can hide BlockDriverState and export it as an fd.
int vdisk_open(path, format, flags)
size_t vdisk_pread(fd, buf, size, offset)
size_t vdisk_pwrite(fd, buff, size, offset)
int vdisk_close(fd)

int vdisk_get_size(fd)

That way no internal structures are exported and we use a minimal set 
of functions that are very unlikely to change.
There is no support for snapshots, metadata etc. But these APIs can be 
added later.

And of-course we can always define the lib as experimental until the 
API stabilizes.
Anthony Liguori - Aug. 24, 2011, 12:50 p.m.
On 08/24/2011 06:32 AM, Saggi Mizrahi wrote:
> On Tue 23 Aug 2011 07:21:56 PM IDT, Anthony Liguori wrote:
>> But QEMU is GPL. Libraries derived from QEMU will also be GPL.
>>
>> Regards,
>>
>> Anthony Liguori
>>
>>>
>>> Regards,
>>> Daniel
>
> OK, I admit it was a pretty naive solution. But I always try to do the
> simplest solution first.
> The license issues can be solved later. (Having it as GPL later changing
> to LGPL if we can).
>
> As for the API I can create start a specialized API for the lib that
> uses the internal API.
> I can hide BlockDriverState and export it as an fd.
> int vdisk_open(path, format, flags)
> size_t vdisk_pread(fd, buf, size, offset)
> size_t vdisk_pwrite(fd, buff, size, offset)
> int vdisk_close(fd)
>
> int vdisk_get_size(fd)

That's not really much better.  You'll only end up reinventing the block 
layer API.

The best route is to focus on cleaning up the block layer interface. 
This means converting existing users to use a single interface, adding 
documentation to each interface, writing test cases against the block 
layer API.

Practically speaking, I think we really need to move the block layer to 
use glib primitives too before you can realistically consume the code 
outside of QEMU.

Regards,

Anthony Liguori

>
> That way no internal structures are exported and we use a minimal set of
> functions that are very unlikely to change.
> There is no support for snapshots, metadata etc. But these APIs can be
> added later.
>
> And of-course we can always define the lib as experimental until the API
> stabilizes.
>
Daniel P. Berrange - Aug. 24, 2011, 12:55 p.m.
On Wed, Aug 24, 2011 at 07:50:37AM -0500, Anthony Liguori wrote:
> On 08/24/2011 06:32 AM, Saggi Mizrahi wrote:
> >On Tue 23 Aug 2011 07:21:56 PM IDT, Anthony Liguori wrote:
> >>But QEMU is GPL. Libraries derived from QEMU will also be GPL.
> >>
> >>Regards,
> >>
> >>Anthony Liguori
> >>
> >>>
> >>>Regards,
> >>>Daniel
> >
> >OK, I admit it was a pretty naive solution. But I always try to do the
> >simplest solution first.
> >The license issues can be solved later. (Having it as GPL later changing
> >to LGPL if we can).
> >
> >As for the API I can create start a specialized API for the lib that
> >uses the internal API.
> >I can hide BlockDriverState and export it as an fd.
> >int vdisk_open(path, format, flags)
> >size_t vdisk_pread(fd, buf, size, offset)
> >size_t vdisk_pwrite(fd, buff, size, offset)
> >int vdisk_close(fd)
> >
> >int vdisk_get_size(fd)
> 
> That's not really much better.  You'll only end up reinventing the
> block layer API.
> 
> The best route is to focus on cleaning up the block layer interface.
> This means converting existing users to use a single interface,
> adding documentation to each interface, writing test cases against
> the block layer API.
> 
> Practically speaking, I think we really need to move the block layer
> to use glib primitives too before you can realistically consume the
> code outside of QEMU.

I think it'd be interesting to ask what the intended use cases for
accessing the block layer API outside of QEMU are ?

Personally, for libvirt, I'm only interested in a libblkid like library
that lets us detect what format a disk is, and query some basic metadata
about the file (logical size, vs physical size, backing file paths,
backing file format, whether encryption is present, etc - qemu-img info
equivalent basically).

Apps that actually want to read/write the contents of a virtual disk,
may well be better off just using libguestfs as the API, since they
may well want to actually interpret the data inside the disk, eg to
read specific files out of the filesystem, etc

There's possibly a need for a API to create/clone/convert virtual
disks formats, but that could likely be done without directly
exposing apps to any of the low level BlockDrv APis like pread/pwrite,
instead providing an API at the level of 'qemu-img create' command
args.

Regards,
Daniel

Patch

diff --git a/.gitignore b/.gitignore
index 59c343c..a389059 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,5 +1,4 @@ 
-config-devices.*
-config-all-devices.*
+config-devices.* config-all-devices.*
 config-host.*
 config-target.*
 trace.h
@@ -15,6 +14,7 @@  libdis*
 libhw32
 libhw64
 libuser
+libvdisk
 qapi-generated
 qemu-doc.html
 qemu-tech.html
diff --git a/Makefile.objs b/Makefile.objs
index 432b619..291f194 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -171,6 +171,15 @@  common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o
 common-obj-$(CONFIG_XEN_BACKEND) += xen_console.o xenfb.o xen_disk.o xen_nic.o
 
 ######################################################################
+# libvdisk
+
+vdisk-obj-y = $(block-obj-y)
+
+vdisk-obj-y += qemu-tool.o qemu-error.o
+vdisk-obj-y += $(oslib-obj-y) $(trace-obj-y) $(block-obj-y)
+vdisk-obj-y += $(qobject-obj-y) $(version-obj-y) qemu-timer-common.o
+
+######################################################################
 # libuser
 
 user-obj-y =