Re: [Qemu-devel] [PATCH] block: add read only to whitelist
Am 28.05.2013 um 08:44 hat Fam Zheng geschrieben: We may want to include a driver in the whitelist for read only tasks such as diagnosing or exporting guest data (with libguestfs as a good example). This patch introduces the magic prefix ^ to include a driver to the whitelist, but only enables qemu to open specific image format readonly, and returns -ENOTSUP for RW opening. Example: To include vmdk readonly, and others read+write: ./configure --block-drv-whitelist=qcow2,raw,file,qed,^vmdk Signed-off-by: Fam Zheng f...@redhat.com ^ looks a bit like not, so this is potentially confusing syntax. Why not simply add a second option? ./configure --block-drv-whitelist=qcow2,raw,file,qed \ --block-drv-ro-whitelist=vmdk Kevin
Re: [Qemu-devel] [PATCH] block: add read only to whitelist
On Tue, May 28, 2013 at 02:44:26PM +0800, Fam Zheng wrote:
We may want to include a driver in the whitelist for read only tasks
such as diagnosing or exporting guest data (with libguestfs as a good
example). This patch introduces the magic prefix ^ to include a driver
to the whitelist, but only enables qemu to open specific image
format readonly, and returns -ENOTSUP for RW opening.
Example: To include vmdk readonly, and others read+write:
./configure --block-drv-whitelist=qcow2,raw,file,qed,^vmdk
This is great, thanks for tackling this. block/vmdk.c isn't suitable
for running real VMs (read-write) since it's not optimized for
concurrent I/O but it is usable for libguestfs (read-only).
Signed-off-by: Fam Zheng f...@redhat.com
---
block.c | 43 +++
blockdev.c| 4 ++--
configure | 2 ++
include/block/block.h | 3 ++-
scripts/create_config | 9 -
5 files changed, 41 insertions(+), 20 deletions(-)
diff --git a/block.c b/block.c
index 3f87489..e6a7270 100644
--- a/block.c
+++ b/block.c
@@ -328,28 +328,40 @@ BlockDriver *bdrv_find_format(const char *format_name)
return NULL;
}
-static int bdrv_is_whitelisted(BlockDriver *drv)
+static int bdrv_is_whitelisted(BlockDriver *drv, bool read_only)
{
-static const char *whitelist[] = {
-CONFIG_BDRV_WHITELIST
+static const char *whitelist_rw[] = {
+CONFIG_BDRV_WHITELIST_RW
+};
+static const char *whitelist_ro[] = {
+CONFIG_BDRV_WHITELIST_RO
};
const char **p;
Please also make the ./configure lists separate. The funky ^ syntax is
not obvious. Better to have:
./configure --block-drv-whitelist-rw=qcow2,raw,file,qed \
--block-drv-whitelist-ro=vmdk,vpc
-if (!whitelist[0])
+if (!whitelist_rw[0] !whitelist_ro[0]) {
return 1; /* no whitelist, anything goes */
+}
-for (p = whitelist; *p; p++) {
+for (p = whitelist_rw; *p; p++) {
if (!strcmp(drv-format_name, *p)) {
return 1;
}
}
+if (read_only) {
+for (p = whitelist_ro; *p; p++) {
+if (!strcmp(drv-format_name, *p)) {
+return 1;
+}
+}
+}
return 0;
}
-BlockDriver *bdrv_find_whitelisted_format(const char *format_name)
+BlockDriver *bdrv_find_whitelisted_format(const char *format_name,
+ bool read_only)
{
BlockDriver *drv = bdrv_find_format(format_name);
-return drv bdrv_is_whitelisted(drv) ? drv : NULL;
+return drv bdrv_is_whitelisted(drv, read_only) ? drv : NULL;
}
typedef struct CreateCo {
@@ -684,10 +696,6 @@ static int bdrv_open_common(BlockDriverState *bs,
BlockDriverState *file,
trace_bdrv_open_common(bs, filename ?: , flags, drv-format_name);
-if (use_bdrv_whitelist !bdrv_is_whitelisted(drv)) {
-return -ENOTSUP;
-}
-
/* bdrv_open() with directly using a protocol as drv. This layer is
already
* opened, so assign it to bs (while file becomes a closed
BlockDriverState)
* and return immediately. */
if (file != NULL drv-bdrv_file_open) {
bdrv_swap(file, bs);
return 0;
}
I think your change is okay. You moved the check after this early
return, but file is already opened so we passed the whitelist check
already. This is a little tricky but it seems fine.
Stefan
Re: [Qemu-devel] [PATCH] block: add read only to whitelist
Il 28/05/2013 08:44, Fam Zheng ha scritto:
diff --git a/scripts/create_config b/scripts/create_config
index c471e8c..2dfda3e 100755
--- a/scripts/create_config
+++ b/scripts/create_config
@@ -35,11 +35,18 @@ case $line in
echo
;;
CONFIG_BDRV_WHITELIST=*)
-echo #define CONFIG_BDRV_WHITELIST \\
+echo #define CONFIG_BDRV_WHITELIST_RW \\
for drv in ${line#*=}; do
+ [[ ${drv} = ^* ]] continue;
I didn't know about this feature. Can you point me to the documentation?
You would need to change the #! header to #! /bin/bash if you use it,
but since you have to respin anyway, I'd ask you to use case instead. :)
Paolo
echo \${drv}\,\\
done
echo NULL
+echo #define CONFIG_BDRV_WHITELIST_RO \\
+for drv in ${line#*=}; do
+ [[ ${drv} != ^* ]] continue;
+ echo \${drv#^}\,\\
+done
+echo NULL
;;
CONFIG_*=y) # configuration
name=${line%=*}
Re: [Qemu-devel] [PATCH] block: add read only to whitelist
On Tue, 05/28 10:10, Paolo Bonzini wrote:
Il 28/05/2013 08:44, Fam Zheng ha scritto:
diff --git a/scripts/create_config b/scripts/create_config
index c471e8c..2dfda3e 100755
--- a/scripts/create_config
+++ b/scripts/create_config
@@ -35,11 +35,18 @@ case $line in
echo
;;
CONFIG_BDRV_WHITELIST=*)
-echo #define CONFIG_BDRV_WHITELIST \\
+echo #define CONFIG_BDRV_WHITELIST_RW \\
for drv in ${line#*=}; do
+ [[ ${drv} = ^* ]] continue;
I didn't know about this feature. Can you point me to the documentation?
Yes, it is bash only, I'd better use a more compatible way.
http://mywiki.wooledge.org/glob
You would need to change the #! header to #! /bin/bash if you use it,
but since you have to respin anyway, I'd ask you to use case instead. :)
As Stefan and Kevin pointed out, I'll replace ^ prefix with a separate
configure option, it'll become
CONFIG_BDRV_WHITELIST_RW=*)
...
;;
CONFIG_BDRV_WHITELIST_RO=*)
...
;;
Then I won't need globbing.
--
Fam
