[Qemu-devel] [PATCH 1/2] Better support for distros using /lib64 directories

[Qemu-devel] [PATCH 1/2] Better support for distros using /lib64 directories

[Saggi Mizrahi]

Some distributions use lib64 directories for 64 bit libraries. Currently
configure would always generate libdir as $PREFIX/lib. By checking if
the current distro has a /lib64 directory we can assume it is using this
convention.

Signed-off-by: Saggi Mizrahi <smizrahi@redhat.com>
---
 configure |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/configure b/configure
index 4f87e0a..d361e91 100755
--- a/configure
+++ b/configure
@@ -147,6 +147,9 @@ datadir="\${prefix}/share/qemu"
 docdir="\${prefix}/share/doc/qemu"
 bindir="\${prefix}/bin"
 libdir="\${prefix}/lib"
+if [ -d /lib64 ]; then
+libdir="\${prefix}/lib64"
+fi
 includedir="\${prefix}/include"
 sysconfdir="\${prefix}/etc"
 confsuffix="/qemu"
-- 
1.7.7

[Qemu-devel] [PATCH 2/2] [WIP]Added target to build libvdisk

[Saggi Mizrahi]

RFC: This is my suggestion for wrapping the block layer with another
supported API. I added only the bare bones verbs I think we can all agree
on.

Apart from the API please also comment on the Makefile. I kinda stitched
it up and I will appreciate comments on it as well.

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.h>

Signed-off-by: Saggi Mizrahi <smizrahi@redhat.com>
---
 .gitignore        |    3 +-
 Makefile.objs     |    9 ++++
 libvdisk/Makefile |   53 +++++++++++++++++++++
 libvdisk/vdisk.c  |  134 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 libvdisk/vdisk.h  |   27 +++++++++++
 5 files changed, 224 insertions(+), 2 deletions(-)
 create mode 100644 libvdisk/Makefile
 create mode 100644 libvdisk/vdisk.c
 create mode 100644 libvdisk/vdisk.h

diff --git a/.gitignore b/.gitignore
index 6d2acab..7221431 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
diff --git a/Makefile.objs b/Makefile.objs
index 01587c8..a355061 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.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 =
diff --git a/libvdisk/Makefile b/libvdisk/Makefile
new file mode 100644
index 0000000..6b0a28f
--- /dev/null
+++ b/libvdisk/Makefile
@@ -0,0 +1,53 @@
+# Makefile for libvdisk.
+
+include ../config-host.mak
+include $(SRC_PATH)/rules.mak
+BUILD_DIR=$(CURDIR)
+
+.PHONY: all libvdisk block trace install uninstall
+
+$(call set-vpath, $(SRC_PATH))
+
+QEMU_CFLAGS+=-I.. -fPIC -I$(BUILD_DIR)
+
+include $(SRC_PATH)/Makefile.objs
+
+libvdisk: block block.o libvdisk.so.1.0
+
+$(BUILD_DIR)/config-host.mak: ../config-host.mak
+	ln -sf $< $@
+
+# Copy because I build the obj with -fPIC which is not needed
+# for regular qemu build
+block: trace
+	mkdir -p block
+	ln -sf $(SRC_PATH)/block/*.[ch] block
+trace: trace.h
+	mkdir -p trace
+	ln -sf $(SRC_PATH)/trace/*.[chd] trace
+
+libvdisk.so.1.0: $(vdisk-obj-y)
+	$(LD) -shared -soname $@ -o $@ -lc $^
+
+install: libvdisk
+	$(INSTALL) vdisk.h $(PREFIX)$(includedir)
+	$(INSTALL) libvdisk.so.1.0 $(PREFIX)$(libdir)
+	ln -sf $(PREFIX)$(libdir)/libvdisk.so.1.0 $(PREFIX)$(libdir)/libvdisk.so.1
+	ln -sf $(PREFIX)$(libdir)/libvdisk.so.1.0 $(PREFIX)$(libdir)/libvdisk.so
+
+uninstall:
+	rm -rf $(PREFIX)$(includedir)/vdisk
+	rm -f $(PREFIX)$(libdir)/libvdisk.so.*
+
+all: libvdisk
+
+clean:
+	rm -rf block/
+	rm -rf trace/
+	rm -f *.[od]
+	rm -f *.mak
+	rm -f trace*
+	rm -f libvdisk.so.*
+
+# Include automatically generated dependency files
+-include $(wildcard *.d */*.d)
diff --git a/libvdisk/vdisk.c b/libvdisk/vdisk.c
new file mode 100644
index 0000000..d28862d
--- /dev/null
+++ b/libvdisk/vdisk.c
@@ -0,0 +1,134 @@
+#include "vdisk.h"
+
+#include <pthread.h>
+#include "block.h"
+
+typedef struct t_virtual_disk {
+    BlockDriverState *bs;
+    pthread_mutex_t mtx;
+
+} VirtualDisk;
+
+void vdisk_init(void) {
+    bdrv_init();
+}
+
+static int open_bdrv_from_path(const char* path, BlockDriverState** bs,
+        int flags, const char* format) {
+    int rc;
+    BlockDriver* driver;
+
+    *bs = bdrv_new(path);
+    if (format == NULL) {
+        driver = NULL;
+    } else {
+        driver = bdrv_find_format(format);
+        if (driver == NULL) {
+            return -EINVAL;
+        }
+    }
+
+    rc = bdrv_open(*bs, path, flags, driver);
+    if (rc < 0) {
+        bdrv_delete(*bs);
+    }
+    return rc;
+}
+
+int vdisk_create(const char *filename, const char *fmt,
+                 const char *base_filename, const char *base_fmt,
+                 char *options, uint64_t img_size, int flags) {
+
+    return bdrv_img_create(filename, fmt, base_filename, base_fmt,
+            options, img_size, flags);
+}
+
+int vdisk_open(VDD* vdd, const char* pathname, int flags, const char* format) {
+    int rc;
+    VirtualDisk* vd = malloc(sizeof(VirtualDisk));
+    if (!vd) { return -ENOMEM; }
+
+    rc = open_bdrv_from_path(pathname, &vd->bs, flags, format);
+    if (rc < 0) {
+        goto free_vd;
+    }
+
+    rc = pthread_mutex_init(&vd->mtx, NULL);
+    if (rc != 0) {
+        rc = -rc;
+        goto free_bs;
+    }
+
+    vdd->virtual_disk = vd;
+
+    return 0;
+
+free_bs:
+    bdrv_delete(vd->bs);
+free_vd:
+    free(vd);
+    return rc;
+}
+
+ssize_t vdisk_pread(VDD* vdd, void *buf, size_t count, off_t offset) {
+    int rc;
+    uint64_t totalSectors;
+    VirtualDisk* vd = vdd->virtual_disk;
+    if (!vd) {
+        return -EBADF;
+    }
+
+    pthread_mutex_lock(&vd->mtx);
+    bdrv_get_geometry(vd->bs, &totalSectors);
+    if ((totalSectors * 512) <= offset) {
+        rc = 0;
+        goto end;
+    }
+
+    rc = bdrv_pread(vd->bs, offset, buf, count);
+end:
+    pthread_mutex_unlock(&vd->mtx);
+    return rc;
+}
+
+ssize_t vdisk_pwrite(VDD* vdd, const void *buf, size_t count, off_t offset) {
+    int rc;
+    uint64_t totalSectors;
+    VirtualDisk* vd = vdd->virtual_disk;
+    if (!vd) {
+        return -EBADF;
+    }
+
+    pthread_mutex_lock(&vd->mtx);
+    bdrv_get_geometry(vd->bs, &totalSectors);
+    if ((totalSectors * 512) <= offset) {
+        rc = 0;
+        goto end;
+    }
+
+    rc = bdrv_pwrite(vd->bs, offset, buf, count);
+end:
+    pthread_mutex_unlock(&vd->mtx);
+    return rc;
+}
+
+int vdisk_flush(VDD* vdd) {
+    VirtualDisk* vd = vdd->virtual_disk;
+    if (!vd) {
+        return -EBADF;
+    }
+
+    return bdrv_flush(vd->bs);
+}
+
+int vdisk_close(VDD* vdd) {
+    VirtualDisk* vd = vdd->virtual_disk;
+    if (!vd) {
+        return -EBADF;
+    }
+
+    bdrv_flush(vd->bs);
+    bdrv_delete(vd->bs);
+    memset(vdd, 0, sizeof(VDD));
+    return 0;
+}
diff --git a/libvdisk/vdisk.h b/libvdisk/vdisk.h
new file mode 100644
index 0000000..6dba7a3
--- /dev/null
+++ b/libvdisk/vdisk.h
@@ -0,0 +1,27 @@
+#include <unistd.h>
+#include <inttypes.h>
+
+#define VDISK_O_RDWR        0x0002
+#define VDISK_O_SNAPSHOT    0x0008 /* open the file read only and save writes in a snapshot */
+#define VDISK_O_NOCACHE     0x0020 /* do not use the host page cache */
+#define VDISK_O_CACHE_WB    0x0040 /* use write-back caching */
+#define VDISK_O_NATIVE_AIO  0x0080 /* use native AIO instead of the thread pool */
+#define VDISK_O_NO_BACKING  0x0100 /* don't open the backing file */
+#define VDISK_O_NO_FLUSH    0x0200 /* disable flushing on this disk */
+
+#define DEFAULT_MAX_DESCIPTORS 1000
+
+typedef struct t_virtual_disk_descriptor {
+    void* virtual_disk;
+} VDD;
+
+void vdisk_init(void);
+
+int vdisk_create(const char *filename, const char *fmt,
+                 const char *base_filename, const char *base_fmt,
+                 char *options, uint64_t img_size, int flags);
+int vdisk_open(VDD* vdd, const char* pathname, int flags, const char* format);
+ssize_t vdisk_pread(VDD* vdd, void *buf, size_t count, off_t offset);
+ssize_t vdisk_pwrite(VDD* vdd, const void *buf, size_t count, off_t offset);
+int vdisk_flush(VDD* vdd);
+int vdisk_close(VDD* vdd);
-- 
1.7.7

Re: [Qemu-devel] [PATCH 2/2] [WIP]Added target to build libvdisk

[Blue Swirl]

On Sun, Oct 30, 2011 at 16:51, Saggi Mizrahi <smizrahi@redhat.com> wrote:
> RFC: This is my suggestion for wrapping the block layer with another
> supported API. I added only the bare bones verbs I think we can all agree
> on.

It can be expected that the API will only grow: file descriptor
versions, async APIs, thread locks, compat versions etc.

> Apart from the API please also comment on the Makefile. I kinda stitched
> it up and I will appreciate comments on it as well.
>
> 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.

So far, we haven't accepted to export any internals as a library
because of the API and ABI maintenance burdens. Has anything changed?

The licenses of the various files vary, I wonder what conditions
should the shared library have.

> To use install the lib and #include <vdisk.h>
>
> Signed-off-by: Saggi Mizrahi <smizrahi@redhat.com>
> ---
>  .gitignore        |    3 +-
>  Makefile.objs     |    9 ++++
>  libvdisk/Makefile |   53 +++++++++++++++++++++
>  libvdisk/vdisk.c  |  134 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  libvdisk/vdisk.h  |   27 +++++++++++
>  5 files changed, 224 insertions(+), 2 deletions(-)
>  create mode 100644 libvdisk/Makefile
>  create mode 100644 libvdisk/vdisk.c
>  create mode 100644 libvdisk/vdisk.h
>
> diff --git a/.gitignore b/.gitignore
> index 6d2acab..7221431 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
> diff --git a/Makefile.objs b/Makefile.objs
> index 01587c8..a355061 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.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 =
> diff --git a/libvdisk/Makefile b/libvdisk/Makefile
> new file mode 100644
> index 0000000..6b0a28f
> --- /dev/null
> +++ b/libvdisk/Makefile
> @@ -0,0 +1,53 @@
> +# Makefile for libvdisk.
> +
> +include ../config-host.mak
> +include $(SRC_PATH)/rules.mak
> +BUILD_DIR=$(CURDIR)
> +
> +.PHONY: all libvdisk block trace install uninstall
> +
> +$(call set-vpath, $(SRC_PATH))
> +
> +QEMU_CFLAGS+=-I.. -fPIC -I$(BUILD_DIR)
> +
> +include $(SRC_PATH)/Makefile.objs
> +
> +libvdisk: block block.o libvdisk.so.1.0

A variable should be defined for 1.0 and used here and elsewhere.

> +
> +$(BUILD_DIR)/config-host.mak: ../config-host.mak
> +       ln -sf $< $@
> +
> +# Copy because I build the obj with -fPIC which is not needed
> +# for regular qemu build
> +block: trace
> +       mkdir -p block
> +       ln -sf $(SRC_PATH)/block/*.[ch] block
> +trace: trace.h
> +       mkdir -p trace
> +       ln -sf $(SRC_PATH)/trace/*.[chd] trace
> +
> +libvdisk.so.1.0: $(vdisk-obj-y)
> +       $(LD) -shared -soname $@ -o $@ -lc $^
> +
> +install: libvdisk
> +       $(INSTALL) vdisk.h $(PREFIX)$(includedir)
> +       $(INSTALL) libvdisk.so.1.0 $(PREFIX)$(libdir)
> +       ln -sf $(PREFIX)$(libdir)/libvdisk.so.1.0 $(PREFIX)$(libdir)/libvdisk.so.1
> +       ln -sf $(PREFIX)$(libdir)/libvdisk.so.1.0 $(PREFIX)$(libdir)/libvdisk.so
> +
> +uninstall:
> +       rm -rf $(PREFIX)$(includedir)/vdisk
> +       rm -f $(PREFIX)$(libdir)/libvdisk.so.*
> +
> +all: libvdisk
> +
> +clean:
> +       rm -rf block/
> +       rm -rf trace/
> +       rm -f *.[od]
> +       rm -f *.mak
> +       rm -f trace*
> +       rm -f libvdisk.so.*
> +
> +# Include automatically generated dependency files
> +-include $(wildcard *.d */*.d)
> diff --git a/libvdisk/vdisk.c b/libvdisk/vdisk.c
> new file mode 100644
> index 0000000..d28862d
> --- /dev/null
> +++ b/libvdisk/vdisk.c
> @@ -0,0 +1,134 @@

There is no header, please specify the license.

> +#include "vdisk.h"
> +
> +#include <pthread.h>
> +#include "block.h"
> +
> +typedef struct t_virtual_disk {
> +    BlockDriverState *bs;
> +    pthread_mutex_t mtx;
> +
> +} VirtualDisk;
> +
> +void vdisk_init(void) {
> +    bdrv_init();
> +}
> +
> +static int open_bdrv_from_path(const char* path, BlockDriverState** bs,

Asterisks should reside close to the variable name, not the type.

> +        int flags, const char* format) {
> +    int rc;
> +    BlockDriver* driver;
> +
> +    *bs = bdrv_new(path);
> +    if (format == NULL) {
> +        driver = NULL;
> +    } else {
> +        driver = bdrv_find_format(format);
> +        if (driver == NULL) {
> +            return -EINVAL;
> +        }
> +    }
> +
> +    rc = bdrv_open(*bs, path, flags, driver);
> +    if (rc < 0) {
> +        bdrv_delete(*bs);
> +    }
> +    return rc;
> +}
> +
> +int vdisk_create(const char *filename, const char *fmt,
> +                 const char *base_filename, const char *base_fmt,
> +                 char *options, uint64_t img_size, int flags) {
> +
> +    return bdrv_img_create(filename, fmt, base_filename, base_fmt,
> +            options, img_size, flags);
> +}
> +
> +int vdisk_open(VDD* vdd, const char* pathname, int flags, const char* format) {
> +    int rc;
> +    VirtualDisk* vd = malloc(sizeof(VirtualDisk));
> +    if (!vd) { return -ENOMEM; }
> +
> +    rc = open_bdrv_from_path(pathname, &vd->bs, flags, format);
> +    if (rc < 0) {
> +        goto free_vd;
> +    }
> +
> +    rc = pthread_mutex_init(&vd->mtx, NULL);

I wonder also what thread API should be used, qemu_thread versions
used internally or standard ones. It would be logical to use
qemu_thread versions but then the external user should use those too.

> +    if (rc != 0) {
> +        rc = -rc;
> +        goto free_bs;
> +    }
> +
> +    vdd->virtual_disk = vd;
> +
> +    return 0;
> +
> +free_bs:
> +    bdrv_delete(vd->bs);
> +free_vd:
> +    free(vd);
> +    return rc;
> +}
> +
> +ssize_t vdisk_pread(VDD* vdd, void *buf, size_t count, off_t offset) {
> +    int rc;
> +    uint64_t totalSectors;
> +    VirtualDisk* vd = vdd->virtual_disk;
> +    if (!vd) {
> +        return -EBADF;
> +    }
> +
> +    pthread_mutex_lock(&vd->mtx);
> +    bdrv_get_geometry(vd->bs, &totalSectors);
> +    if ((totalSectors * 512) <= offset) {
> +        rc = 0;
> +        goto end;
> +    }
> +
> +    rc = bdrv_pread(vd->bs, offset, buf, count);
> +end:
> +    pthread_mutex_unlock(&vd->mtx);
> +    return rc;
> +}
> +
> +ssize_t vdisk_pwrite(VDD* vdd, const void *buf, size_t count, off_t offset) {
> +    int rc;
> +    uint64_t totalSectors;
> +    VirtualDisk* vd = vdd->virtual_disk;
> +    if (!vd) {
> +        return -EBADF;
> +    }
> +
> +    pthread_mutex_lock(&vd->mtx);
> +    bdrv_get_geometry(vd->bs, &totalSectors);
> +    if ((totalSectors * 512) <= offset) {
> +        rc = 0;
> +        goto end;
> +    }
> +
> +    rc = bdrv_pwrite(vd->bs, offset, buf, count);
> +end:
> +    pthread_mutex_unlock(&vd->mtx);
> +    return rc;
> +}
> +
> +int vdisk_flush(VDD* vdd) {
> +    VirtualDisk* vd = vdd->virtual_disk;
> +    if (!vd) {
> +        return -EBADF;
> +    }
> +
> +    return bdrv_flush(vd->bs);
> +}
> +
> +int vdisk_close(VDD* vdd) {
> +    VirtualDisk* vd = vdd->virtual_disk;
> +    if (!vd) {
> +        return -EBADF;
> +    }
> +
> +    bdrv_flush(vd->bs);
> +    bdrv_delete(vd->bs);
> +    memset(vdd, 0, sizeof(VDD));
> +    return 0;
> +}
> diff --git a/libvdisk/vdisk.h b/libvdisk/vdisk.h
> new file mode 100644
> index 0000000..6dba7a3
> --- /dev/null
> +++ b/libvdisk/vdisk.h
> @@ -0,0 +1,27 @@

License header missing.

> +#include <unistd.h>
> +#include <inttypes.h>
> +
> +#define VDISK_O_RDWR        0x0002
> +#define VDISK_O_SNAPSHOT    0x0008 /* open the file read only and save writes in a snapshot */
> +#define VDISK_O_NOCACHE     0x0020 /* do not use the host page cache */
> +#define VDISK_O_CACHE_WB    0x0040 /* use write-back caching */
> +#define VDISK_O_NATIVE_AIO  0x0080 /* use native AIO instead of the thread pool */
> +#define VDISK_O_NO_BACKING  0x0100 /* don't open the backing file */
> +#define VDISK_O_NO_FLUSH    0x0200 /* disable flushing on this disk */
> +
> +#define DEFAULT_MAX_DESCIPTORS 1000

DESCRIPTORS, but actually it is unused.

> +
> +typedef struct t_virtual_disk_descriptor {

t_virtual_disk_descriptor should be VDD.

> +    void* virtual_disk;
> +} VDD;
> +
> +void vdisk_init(void);
> +
> +int vdisk_create(const char *filename, const char *fmt,
> +                 const char *base_filename, const char *base_fmt,
> +                 char *options, uint64_t img_size, int flags);
> +int vdisk_open(VDD* vdd, const char* pathname, int flags, const char* format);
> +ssize_t vdisk_pread(VDD* vdd, void *buf, size_t count, off_t offset);
> +ssize_t vdisk_pwrite(VDD* vdd, const void *buf, size_t count, off_t offset);
> +int vdisk_flush(VDD* vdd);
> +int vdisk_close(VDD* vdd);
> --
> 1.7.7
>
>
>

Re: [Qemu-devel] [PATCH 2/2] [WIP]Added target to build libvdisk

[Saggi Mizrahi]

On Sun 30 Oct 2011 07:21:13 PM IST, Blue Swirl wrote:
> On Sun, Oct 30, 2011 at 16:51, Saggi Mizrahi<smizrahi@redhat.com>  wrote:
>> RFC: This is my suggestion for wrapping the block layer with another
>> supported API. I added only the bare bones verbs I think we can all agree
>> on.
>
> It can be expected that the API will only grow: file descriptor
> versions, async APIs, thread locks, compat versions etc.
There is no doubt it's going to grow but I intend this to be a tools 
API rather then a whole library to write qemu alternatives. My desire 
is to expose basic functionality so other tools (like VDSM) can do 
image manipulation\conversion\inspection. It is not intended to be 
highly optimized but rather simple to use. Further more I suggest it as 
a guideline to have it not interfere with qemu's internals so they can 
remain optimized.
>
>> Apart from the API please also comment on the Makefile. I kinda stitched
>> it up and I will appreciate comments on it as well.
>>
>> 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.
>
> So far, we haven't accepted to export any internals as a library
> because of the API and ABI maintenance burdens. Has anything changed?
In VDSM we want to be able to manipulate images and using qemu-img just 
doesn't cut it. I don't intend to export the internal API, rather I 
would like to create an alternative simple API that is easier to 
maintain that can be consumed by 3rd party. I tried to make the library 
independent from internal qemu data structures and low level 
functinality so it can be easily maintained.
>
> The licenses of the various files vary, I wonder what conditions
> should the shared library have.
For VDSM GPL is good enough (VDSM being GPL). If there are other 
parties interested in making it a more relaxed licence I'm sure there 
is way to pull something off. This can always happen down the road. I 
know that libvirt would like it to be LGPL. In any case I will submit 
my files as LGPL.
>
>> To use install the lib and #include<vdisk.h>
>>
>> Signed-off-by: Saggi Mizrahi<smizrahi@redhat.com>
>> ---
>>   .gitignore        |    3 +-
>>   Makefile.objs     |    9 ++++
>>   libvdisk/Makefile |   53 +++++++++++++++++++++
>>   libvdisk/vdisk.c  |  134 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   libvdisk/vdisk.h  |   27 +++++++++++
>>   5 files changed, 224 insertions(+), 2 deletions(-)
>>   create mode 100644 libvdisk/Makefile
>>   create mode 100644 libvdisk/vdisk.c
>>   create mode 100644 libvdisk/vdisk.h
>>
>> diff --git a/.gitignore b/.gitignore
>> index 6d2acab..7221431 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
>> diff --git a/Makefile.objs b/Makefile.objs
>> index 01587c8..a355061 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.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 =
>> diff --git a/libvdisk/Makefile b/libvdisk/Makefile
>> new file mode 100644
>> index 0000000..6b0a28f
>> --- /dev/null
>> +++ b/libvdisk/Makefile
>> @@ -0,0 +1,53 @@
>> +# Makefile for libvdisk.
>> +
>> +include ../config-host.mak
>> +include $(SRC_PATH)/rules.mak
>> +BUILD_DIR=$(CURDIR)
>> +
>> +.PHONY: all libvdisk block trace install uninstall
>> +
>> +$(call set-vpath, $(SRC_PATH))
>> +
>> +QEMU_CFLAGS+=-I.. -fPIC -I$(BUILD_DIR)
>> +
>> +include $(SRC_PATH)/Makefile.objs
>> +
>> +libvdisk: block block.o libvdisk.so.1.0
>
> A variable should be defined for 1.0 and used here and elsewhere.
>
>> +
>> +$(BUILD_DIR)/config-host.mak: ../config-host.mak
>> +       ln -sf $<  $@
>> +
>> +# Copy because I build the obj with -fPIC which is not needed
>> +# for regular qemu build
>> +block: trace
>> +       mkdir -p block
>> +       ln -sf $(SRC_PATH)/block/*.[ch] block
>> +trace: trace.h
>> +       mkdir -p trace
>> +       ln -sf $(SRC_PATH)/trace/*.[chd] trace
>> +
>> +libvdisk.so.1.0: $(vdisk-obj-y)
>> +       $(LD) -shared -soname $@ -o $@ -lc $^
>> +
>> +install: libvdisk
>> +       $(INSTALL) vdisk.h $(PREFIX)$(includedir)
>> +       $(INSTALL) libvdisk.so.1.0 $(PREFIX)$(libdir)
>> +       ln -sf $(PREFIX)$(libdir)/libvdisk.so.1.0 $(PREFIX)$(libdir)/libvdisk.so.1
>> +       ln -sf $(PREFIX)$(libdir)/libvdisk.so.1.0 $(PREFIX)$(libdir)/libvdisk.so
>> +
>> +uninstall:
>> +       rm -rf $(PREFIX)$(includedir)/vdisk
>> +       rm -f $(PREFIX)$(libdir)/libvdisk.so.*
>> +
>> +all: libvdisk
>> +
>> +clean:
>> +       rm -rf block/
>> +       rm -rf trace/
>> +       rm -f *.[od]
>> +       rm -f *.mak
>> +       rm -f trace*
>> +       rm -f libvdisk.so.*
>> +
>> +# Include automatically generated dependency files
>> +-include $(wildcard *.d */*.d)
>> diff --git a/libvdisk/vdisk.c b/libvdisk/vdisk.c
>> new file mode 100644
>> index 0000000..d28862d
>> --- /dev/null
>> +++ b/libvdisk/vdisk.c
>> @@ -0,0 +1,134 @@
>
> There is no header, please specify the license.
>
>> +#include "vdisk.h"
>> +
>> +#include<pthread.h>
>> +#include "block.h"
>> +
>> +typedef struct t_virtual_disk {
>> +    BlockDriverState *bs;
>> +    pthread_mutex_t mtx;
>> +
>> +} VirtualDisk;
>> +
>> +void vdisk_init(void) {
>> +    bdrv_init();
>> +}
>> +
>> +static int open_bdrv_from_path(const char* path, BlockDriverState** bs,
>
> Asterisks should reside close to the variable name, not the type.
>
>> +        int flags, const char* format) {
>> +    int rc;
>> +    BlockDriver* driver;
>> +
>> +    *bs = bdrv_new(path);
>> +    if (format == NULL) {
>> +        driver = NULL;
>> +    } else {
>> +        driver = bdrv_find_format(format);
>> +        if (driver == NULL) {
>> +            return -EINVAL;
>> +        }
>> +    }
>> +
>> +    rc = bdrv_open(*bs, path, flags, driver);
>> +    if (rc<  0) {
>> +        bdrv_delete(*bs);
>> +    }
>> +    return rc;
>> +}
>> +
>> +int vdisk_create(const char *filename, const char *fmt,
>> +                 const char *base_filename, const char *base_fmt,
>> +                 char *options, uint64_t img_size, int flags) {
>> +
>> +    return bdrv_img_create(filename, fmt, base_filename, base_fmt,
>> +            options, img_size, flags);
>> +}
>> +
>> +int vdisk_open(VDD* vdd, const char* pathname, int flags, const char* format) {
>> +    int rc;
>> +    VirtualDisk* vd = malloc(sizeof(VirtualDisk));
>> +    if (!vd) { return -ENOMEM; }
>> +
>> +    rc = open_bdrv_from_path(pathname,&vd->bs, flags, format);
>> +    if (rc<  0) {
>> +        goto free_vd;
>> +    }
>> +
>> +    rc = pthread_mutex_init(&vd->mtx, NULL);
>
> I wonder also what thread API should be used, qemu_thread versions
> used internally or standard ones. It would be logical to use
> qemu_thread versions but then the external user should use those too.
I would prefer not to use qemu's threading as it'll expose internal 
APIs. I suggest glib's threading,
>
>> +    if (rc != 0) {
>> +        rc = -rc;
>> +        goto free_bs;
>> +    }
>> +
>> +    vdd->virtual_disk = vd;
>> +
>> +    return 0;
>> +
>> +free_bs:
>> +    bdrv_delete(vd->bs);
>> +free_vd:
>> +    free(vd);
>> +    return rc;
>> +}
>> +
>> +ssize_t vdisk_pread(VDD* vdd, void *buf, size_t count, off_t offset) {
>> +    int rc;
>> +    uint64_t totalSectors;
>> +    VirtualDisk* vd = vdd->virtual_disk;
>> +    if (!vd) {
>> +        return -EBADF;
>> +    }
>> +
>> +    pthread_mutex_lock(&vd->mtx);
>> +    bdrv_get_geometry(vd->bs,&totalSectors);
>> +    if ((totalSectors * 512)<= offset) {
>> +        rc = 0;
>> +        goto end;
>> +    }
>> +
>> +    rc = bdrv_pread(vd->bs, offset, buf, count);
>> +end:
>> +    pthread_mutex_unlock(&vd->mtx);
>> +    return rc;
>> +}
>> +
>> +ssize_t vdisk_pwrite(VDD* vdd, const void *buf, size_t count, off_t offset) {
>> +    int rc;
>> +    uint64_t totalSectors;
>> +    VirtualDisk* vd = vdd->virtual_disk;
>> +    if (!vd) {
>> +        return -EBADF;
>> +    }
>> +
>> +    pthread_mutex_lock(&vd->mtx);
>> +    bdrv_get_geometry(vd->bs,&totalSectors);
>> +    if ((totalSectors * 512)<= offset) {
>> +        rc = 0;
>> +        goto end;
>> +    }
>> +
>> +    rc = bdrv_pwrite(vd->bs, offset, buf, count);
>> +end:
>> +    pthread_mutex_unlock(&vd->mtx);
>> +    return rc;
>> +}
>> +
>> +int vdisk_flush(VDD* vdd) {
>> +    VirtualDisk* vd = vdd->virtual_disk;
>> +    if (!vd) {
>> +        return -EBADF;
>> +    }
>> +
>> +    return bdrv_flush(vd->bs);
>> +}
>> +
>> +int vdisk_close(VDD* vdd) {
>> +    VirtualDisk* vd = vdd->virtual_disk;
>> +    if (!vd) {
>> +        return -EBADF;
>> +    }
>> +
>> +    bdrv_flush(vd->bs);
>> +    bdrv_delete(vd->bs);
>> +    memset(vdd, 0, sizeof(VDD));
>> +    return 0;
>> +}
>> diff --git a/libvdisk/vdisk.h b/libvdisk/vdisk.h
>> new file mode 100644
>> index 0000000..6dba7a3
>> --- /dev/null
>> +++ b/libvdisk/vdisk.h
>> @@ -0,0 +1,27 @@
>
> License header missing.
>
>> +#include<unistd.h>
>> +#include<inttypes.h>
>> +
>> +#define VDISK_O_RDWR        0x0002
>> +#define VDISK_O_SNAPSHOT    0x0008 /* open the file read only and save writes in a snapshot */
>> +#define VDISK_O_NOCACHE     0x0020 /* do not use the host page cache */
>> +#define VDISK_O_CACHE_WB    0x0040 /* use write-back caching */
>> +#define VDISK_O_NATIVE_AIO  0x0080 /* use native AIO instead of the thread pool */
>> +#define VDISK_O_NO_BACKING  0x0100 /* don't open the backing file */
>> +#define VDISK_O_NO_FLUSH    0x0200 /* disable flushing on this disk */
>> +
>> +#define DEFAULT_MAX_DESCIPTORS 1000
>
> DESCRIPTORS, but actually it is unused.
>
>> +
>> +typedef struct t_virtual_disk_descriptor {
>
> t_virtual_disk_descriptor should be VDD.
>
>> +    void* virtual_disk;
>> +} VDD;
>> +
>> +void vdisk_init(void);
>> +
>> +int vdisk_create(const char *filename, const char *fmt,
>> +                 const char *base_filename, const char *base_fmt,
>> +                 char *options, uint64_t img_size, int flags);
>> +int vdisk_open(VDD* vdd, const char* pathname, int flags, const char* format);
>> +ssize_t vdisk_pread(VDD* vdd, void *buf, size_t count, off_t offset);
>> +ssize_t vdisk_pwrite(VDD* vdd, const void *buf, size_t count, off_t offset);
>> +int vdisk_flush(VDD* vdd);
>> +int vdisk_close(VDD* vdd);
>> --
>> 1.7.7
>>
>>
>>
Thank you, All comments will be addressed. But I would like to get a 
green light for the whole process before sending another version.

Re: [Qemu-devel] [PATCH 2/2] [WIP]Added target to build libvdisk

[Paolo Bonzini]

On 10/30/2011 07:04 PM, Saggi Mizrahi wrote:
>>
>> I wonder also what thread API should be used, qemu_thread versions
>> used internally or standard ones. It would be logical to use
>> qemu_thread versions but then the external user should use those too.
> I would prefer not to use qemu's threading as it'll expose internal
> APIs. I suggest glib's threading,

Is it visible in any way to the client?  Also, perhaps thread-safety 
should be left to the client so that they can use whatever API they like?

Paolo

Re: [Qemu-devel] [PATCH 1/2] Better support for distros using /lib64 directories

[Paolo Bonzini]

On 10/30/2011 05:51 PM, Saggi Mizrahi wrote:
> Some distributions use lib64 directories for 64 bit libraries. Currently
> configure would always generate libdir as $PREFIX/lib. By checking if
> the current distro has a /lib64 directory we can assume it is using this
> convention.

This doesn't work if people compile a 32-bit version of QEMU.  The right 
directory could be obtained with "gcc --print-multi-os-directory 
$CFLAGS", but I'm not sure whether it is a good thing to do.  We should 
try to avoid gratuitous differences from Autoconf.

The right thing to do would be to tweak the command-line supported by 
configure to the point that packagers can just use the same command-line 
that Autoconf uses (in RPM parlance that would be "%configure").

Paolo