* [Qemu-devel] [PATCH V4 0/5] Redefine and export backing file loop check
@ 2013-07-22 8:05 Xu Wang
2013-07-22 8:05 ` [Qemu-devel] [PATCH V4 1/5] block/qemu-img: Refine and export infinite loop checking in collect_image_info_list() Xu Wang
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Xu Wang @ 2013-07-22 8:05 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha, Xu Wang, famz, wdongxu
If there is loop exists in the backing file chain, many problems
could be caused by it, such as no response and segment fault during
system boot. Hence stopping backing file loop appear is very necessary.
These patches refine and export loop checking function from collect_image_
info_list() to block.c and build a independent function named bdrv_
backing_file_loop_check(). Backing file loop checking is added before
image created, before change backing file and before system boot.
Updates from V3:
1. Comments fix for function bdrv_backing_file_loop_check().
2. Add ret check for fseek()/fread() in get_lnk_target_file().
3. Add limit of shortcuts filename length reading during comparing.
4. Add error_report() in driv_init().
5. Remove redundant loop check in qcow2/qed_change_backing_file().
Updates from V2:
1. Removed parameter @chain from bdrv_backing_file_loop_check()
2. Comments and format fix, all patches were checked by checkpatch.pl
3. Fixed *bs leak.
4. Improved logic of .lnk file recognization.
5. Add filename lenth limit check in while()
6. Changed get_win_inode() to get_inode() and move all inode get method
into it to make logic more simpler.
7. Added value of @fmt as suggested.
8. Added backing file loop check in qcow2.c/qed.c
Xu Wang (5):
block/qemu-img: Refine and export infinite loop checking in
collect_image_info_list()
block: Add WIN32 platform support for backing_file_loop_check()
block: Check infinite loop in bdrv_img_create()
block: Add backing file loop check in change_backing_file()
block: Add infinite loop check in drive_init()
block.c | 257 +++++++++++++++++++++++++++++++++++++++++++++++++-
blockdev.c | 7 ++
include/block/block.h | 4 +
qemu-img.c | 29 ++----
4 files changed, 272 insertions(+), 25 deletions(-)
--
1.8.1.4
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH V4 1/5] block/qemu-img: Refine and export infinite loop checking in collect_image_info_list()
2013-07-22 8:05 [Qemu-devel] [PATCH V4 0/5] Redefine and export backing file loop check Xu Wang
@ 2013-07-22 8:05 ` Xu Wang
2013-07-22 8:05 ` [Qemu-devel] [PATCH V4 2/5] block: Add WIN32 platform support for backing_file_loop_check() Xu Wang
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Xu Wang @ 2013-07-22 8:05 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha, Xu Wang, famz, wdongxu
If there is a loop exists in the backing file chain, many problems
could be caused by it, such as no response and segment fault during
system boot. Hence stopping backing file loop appear is very necessary.
This patch refine and export loop checking function from collect_image_
info_list() to block.c and build a independent function named bdrv_
backing_file_loop_check().
Signed-off-by: Xu Wang <cngesaint@gmail.com>
---
block.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++
include/block/block.h | 4 +++
qemu-img.c | 29 +++++-----------
3 files changed, 109 insertions(+), 20 deletions(-)
diff --git a/block.c b/block.c
index 183fec8..aea653d 100644
--- a/block.c
+++ b/block.c
@@ -4433,6 +4433,102 @@ bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie *cookie)
bs->total_time_ns[cookie->type] += get_clock() - cookie->start_time_ns;
}
+static gboolean str_equal_func(gconstpointer a, gconstpointer b)
+{
+ return strcmp(a, b) == 0;
+}
+
+/**
+ * Check backing file chain if there is a loop in it.
+ *
+ * @filename: topmost image filename, absolute or relative path is OK.
+ * @fmt: topmost image format (may be NULL to autodetect)
+ * @backing_file: if this value is set, @filename inode (-1 if it doesn't
+ * exist) will insert into hash table directly. Loop check
+ * starts from it. Absolute or relative path is OK for it.
+ * @backing_format: format of backing file
+ *
+ * Returns: true for backing file loop or error happened, false for no loop.
+ */
+bool bdrv_backing_file_loop_check(const char *filename, const char *fmt,
+ const char *backing_file,
+ const char *backing_format) {
+ GHashTable *inodes;
+ BlockDriverState *bs;
+ BlockDriver *drv;
+ struct stat sbuf;
+ long inode = 0;
+ int ret;
+ char fbuf[1024];
+
+ inodes = g_hash_table_new_full(g_str_hash, str_equal_func, NULL, NULL);
+
+ if (backing_file) {
+ /* Check if file exists. */
+ if (access(filename, F_OK)) {
+ inode = -1;
+ } else {
+ if (stat(filename, &sbuf) == -1) {
+ error_report("Get file %s stat failed.", filename);
+ goto err;
+ }
+ inode = (long)sbuf.st_ino;
+ }
+
+ filename = backing_file;
+ fmt = backing_format;
+ g_hash_table_insert(inodes, (gpointer)&inode, NULL);
+ }
+
+ while (filename && (filename[0] != '\0')) {
+ if (stat(filename, &sbuf) == -1) {
+ error_report("Get file %s stat failed.", filename);
+ goto err;
+ }
+ inode = (long)sbuf.st_ino;
+
+ if (g_hash_table_lookup_extended(inodes, &inode, NULL, NULL)) {
+ error_report("Backing file '%s' creates an infinite loop.",
+ filename);
+ goto err;
+ }
+ g_hash_table_insert(inodes, (gpointer)&inode, NULL);
+
+ bs = bdrv_new("image");
+
+ if (fmt) {
+ drv = bdrv_find_format(fmt);
+ if (!drv) {
+ error_report("Unknown file format '%s'", fmt);
+ bdrv_delete(bs);
+ goto err;
+ }
+ } else {
+ drv = NULL;
+ }
+
+ ret = bdrv_open(bs, filename, NULL,
+ BDRV_O_CACHE_WB | BDRV_O_NO_BACKING, drv);
+ if (ret < 0) {
+ error_report("Could not open '%s': %s", filename, strerror(-ret));
+ bdrv_delete(bs);
+ goto err;
+ }
+
+ bdrv_get_backing_filename(bs, fbuf, sizeof(fbuf));
+ filename = fbuf;
+ fmt = NULL;
+
+ bdrv_delete(bs);
+ }
+ g_hash_table_destroy(inodes);
+ return false;
+
+err:
+ g_hash_table_destroy(inodes);
+ return true;
+}
+
void bdrv_img_create(const char *filename, const char *fmt,
const char *base_filename, const char *base_fmt,
char *options, uint64_t img_size, int flags,
diff --git a/include/block/block.h b/include/block/block.h
index dd8eca1..3dc29bb 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -333,6 +333,10 @@ int bdrv_save_vmstate(BlockDriverState *bs, const uint8_t *buf,
int bdrv_load_vmstate(BlockDriverState *bs, uint8_t *buf,
int64_t pos, int size);
+bool bdrv_backing_file_loop_check(const char *filename, const char *fmt,
+ const char *backing_file,
+ const char *backing_format);
+
void bdrv_img_create(const char *filename, const char *fmt,
const char *base_filename, const char *base_fmt,
char *options, uint64_t img_size, int flags,
diff --git a/qemu-img.c b/qemu-img.c
index f8c97d3..19cac5e 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1620,11 +1620,6 @@ static void dump_human_image_info_list(ImageInfoList *list)
}
}
-static gboolean str_equal_func(gconstpointer a, gconstpointer b)
-{
- return strcmp(a, b) == 0;
-}
-
/**
* Open an image file chain and return an ImageInfoList
*
@@ -1642,24 +1637,19 @@ static ImageInfoList *collect_image_info_list(const char *filename,
bool chain)
{
ImageInfoList *head = NULL;
+ BlockDriverState *bs;
+ ImageInfoList *elem;
ImageInfoList **last = &head;
- GHashTable *filenames;
+ ImageInfo *info;
Error *err = NULL;
- filenames = g_hash_table_new_full(g_str_hash, str_equal_func, NULL, NULL);
+ /* check if loop exists and build ImageInfoList */
+ if (chain &&
+ bdrv_backing_file_loop_check(filename, fmt, NULL, NULL)) {
+ goto err;
+ }
while (filename) {
- BlockDriverState *bs;
- ImageInfo *info;
- ImageInfoList *elem;
-
- if (g_hash_table_lookup_extended(filenames, filename, NULL, NULL)) {
- error_report("Backing file '%s' creates an infinite loop.",
- filename);
- goto err;
- }
- g_hash_table_insert(filenames, (gpointer)filename, NULL);
-
bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING,
false, false);
if (!bs) {
@@ -1692,12 +1682,11 @@ static ImageInfoList *collect_image_info_list(const char *filename,
}
}
}
- g_hash_table_destroy(filenames);
+
return head;
err:
qapi_free_ImageInfoList(head);
- g_hash_table_destroy(filenames);
return NULL;
}
--
1.8.1.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH V4 2/5] block: Add WIN32 platform support for backing_file_loop_check()
2013-07-22 8:05 [Qemu-devel] [PATCH V4 0/5] Redefine and export backing file loop check Xu Wang
2013-07-22 8:05 ` [Qemu-devel] [PATCH V4 1/5] block/qemu-img: Refine and export infinite loop checking in collect_image_info_list() Xu Wang
@ 2013-07-22 8:05 ` Xu Wang
2013-07-22 8:40 ` Fam Zheng
2013-07-22 8:05 ` [Qemu-devel] [PATCH V4 3/5] block: Check infinite loop in bdrv_img_create() Xu Wang
` (2 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Xu Wang @ 2013-07-22 8:05 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha, Xu Wang, famz, wdongxu
Method of get_inode is different between Linux and WIN32 plateform.
This patch added inode caculate method on Windows plateform so that
backing file check could work on Windows plateform.
Signed-off-by: Xu Wang <cngesaint@gmail.com>
---
block.c | 160 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 152 insertions(+), 8 deletions(-)
diff --git a/block.c b/block.c
index aea653d..3e23b67 100644
--- a/block.c
+++ b/block.c
@@ -51,6 +51,10 @@
#define NOT_DONE 0x7fffffff /* used while emulated sync operation in progress */
+#ifdef _WIN32
+#define MAX_PATH_LEN 8192
+#endif
+
typedef enum {
BDRV_REQ_COPY_ON_READ = 0x1,
BDRV_REQ_ZERO_WRITE = 0x2,
@@ -4433,6 +4437,147 @@ bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie *cookie)
bs->total_time_ns[cookie->type] += get_clock() - cookie->start_time_ns;
}
+#ifdef _WIN32
+static int get_lnk_target_file(const char *lnk_file, char *filepath)
+{
+ unsigned int flag, offset;
+ unsigned int sflag;
+ char uch;
+ int i = 0;
+
+ FILE *fd = fopen(lnk_file, "rb");
+ if (!fd) {
+ error_report("Open file %s failed.", lnk_file);
+ return -1;
+ }
+
+ /* Check if the file is shortcuts by reading first 4 bytes if it's 0x4c */
+ if (fread(&flag, 4, 1, fd) != 1) {
+ error_report("Read 0x4c field of %s failed.", lnk_file);
+ goto err;
+ }
+
+ if (flag != 0x4c) {
+ error_report("%s is not a lnk file.", lnk_file);
+ goto err;
+ }
+
+ /* Parse flags on offset 0x14 */
+ if (fseek(fd, 0x14, SEEK_SET)) {
+ error_report("Seek flags of %s failed.", lnk_file);
+ goto err;
+ }
+
+ if (fread(&flag, 4, 1, fd) != 1) {
+ error_report("Read flags of %s failed.", lnk_file);
+ goto err;
+ }
+
+ /* seek to the length of item ID list */
+ if (fseek(fd, 0x4c, SEEK_SET)) {
+ error_report("Seek %s length of item ID list failed.", lnk_file);
+ goto err;
+ }
+
+ /* Parse Shell item ID list */
+ if (flag & 0x01) {
+ /* Get the length of SHITEMID */
+ if (fread(&sflag, 2, 1, fd) != 1) {
+ error_report("Read %s length of SHITEMID failed.", lnk_file);
+ goto err;
+ }
+
+ /* Skip SHITEMID */
+ if (fseek(fd, sflag, SEEK_CUR)) {
+ error_report("Skip SHITEMID of %s failed.", lnk_file);
+ goto err;
+ }
+ }
+
+ offset = ftell(fd);
+ if (offset == -1) {
+ error_report("ftell() of %s failed.", lnk_file);
+ goto err;
+ }
+ /* Read offset of local path */
+ if (fseek(fd, offset + 0x10, SEEK_SET)) {
+ error_report("Seek offset of %s failed.", lnk_file);
+ goto err;
+ }
+ if (fread(&flag, 4, 1, fd) != 1) {
+ error_report("Read offset of %s failed.", lnk_file);
+ goto err;
+ }
+ /* Seek to the start of target file path */
+ if (fseek(fd, flag + offset, SEEK_SET)) {
+ error_report("Seek target file path of %s failed.", lnk_file);
+ goto err;
+ }
+
+ do {
+ if (fread(&uch, 1, 1, fd) != 1) {
+ error_report("Read target path of %s failed.", lnk_file);
+ goto err;
+ }
+ filepath[i++] = uch;
+ } while ((i <= MAX_PATH_LEN) && (uch != '\0'));
+
+ fclose(fd);
+ return 0;
+
+err:
+ fclose(fd);
+ return -1;
+}
+#endif
+
+static long get_inode(const char *filename)
+{
+ #ifdef _WIN32
+ char pbuf[MAX_PATH_LEN], *p;
+ long inode;
+ struct stat sbuf;
+ char path[MAX_PATH_LEN];
+ int len;
+
+ /* If filename contains .lnk, it's a shortcuts. Target file
+ * need to be parsed.
+ */
+ len = strlen(filename);
+ if ((len > 4) && (strcmp(filename + len - 4, ".lnk"))) {
+ if (get_lnk_target_file(filename, path)) {
+ error_report("Parse .lnk file %s failed.", filename);
+ return -1;
+ }
+ } else {
+ memcpy(path, filename, sizeof(filename));
+ }
+
+ if (stat(path, &sbuf) == -1) {
+ error_report("get file %s stat error.", path);
+ return -1;
+ }
+ if (GetFullPathName(path, MAX_PATH_LEN, pbuf, &p) != 0) {
+ inode = 11003;
+ for (p = pbuf; *p != '\0'; p++) {
+ inode = inode * 31 + *(unsigned char *)p;
+ }
+ return (inode * 911) & 0x7FFF;
+ }
+
+ return -1;
+ #else
+ struct stat sbuf;
+
+ if (stat(filename, &sbuf) == -1) {
+ error_report("get file %s stat error.", filename);
+ return -1;
+ }
+
+ return sbuf.st_ino;
+ #endif
+}
+
static gboolean str_equal_func(gconstpointer a, gconstpointer b)
{
return strcmp(a, b) == 0;
@@ -4456,7 +4601,6 @@ bool bdrv_backing_file_loop_check(const char *filename, const char *fmt,
GHashTable *inodes;
BlockDriverState *bs;
BlockDriver *drv;
- struct stat sbuf;
long inode = 0;
int ret;
char fbuf[1024];
@@ -4468,11 +4612,11 @@ bool bdrv_backing_file_loop_check(const char *filename, const char *fmt,
if (access(filename, F_OK)) {
inode = -1;
} else {
- if (stat(filename, &sbuf) == -1) {
- error_report("Get file %s stat failed.", filename);
+ inode = get_inode(filename);
+ if (inode == -1) {
+ error_report("Get file %s inode failed.", filename);
goto err;
}
- inode = (long)sbuf.st_ino;
}
filename = backing_file;
@@ -4481,11 +4625,11 @@ bool bdrv_backing_file_loop_check(const char *filename, const char *fmt,
}
while (filename && (filename[0] != '\0')) {
- if (stat(filename, &sbuf) == -1) {
- error_report("Get file %s stat failed.", filename);
- goto err;
+ inode = get_inode(filename);
+ if (inode == -1) {
+ error_report("Get file %s inode failed.", filename);
+ goto err;
}
- inode = (long)sbuf.st_ino;
if (g_hash_table_lookup_extended(inodes, &inode, NULL, NULL)) {
error_report("Backing file '%s' creates an infinite loop.",
--
1.8.1.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH V4 3/5] block: Check infinite loop in bdrv_img_create()
2013-07-22 8:05 [Qemu-devel] [PATCH V4 0/5] Redefine and export backing file loop check Xu Wang
2013-07-22 8:05 ` [Qemu-devel] [PATCH V4 1/5] block/qemu-img: Refine and export infinite loop checking in collect_image_info_list() Xu Wang
2013-07-22 8:05 ` [Qemu-devel] [PATCH V4 2/5] block: Add WIN32 platform support for backing_file_loop_check() Xu Wang
@ 2013-07-22 8:05 ` Xu Wang
2013-07-22 8:44 ` Fam Zheng
2013-07-22 8:05 ` [Qemu-devel] [PATCH V4 4/5] block: Add backing file loop check in change_backing_file() Xu Wang
2013-07-22 8:05 ` [Qemu-devel] [PATCH V4 5/5] block: Add infinite loop check in drive_init() Xu Wang
4 siblings, 1 reply; 9+ messages in thread
From: Xu Wang @ 2013-07-22 8:05 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha, Xu Wang, famz, wdongxu
Backing file loop should be checked before qemu-img create command
execution. If loop was found, qemu-img create should be stopped and
an error was printed.
Signed-off-by: Xu Wang <cngesaint@gmail.com>
---
block.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/block.c b/block.c
index 3e23b67..9daf63c 100644
--- a/block.c
+++ b/block.c
@@ -4735,15 +4735,15 @@ void bdrv_img_create(const char *filename, const char *fmt,
}
backing_file = get_option_parameter(param, BLOCK_OPT_BACKING_FILE);
+ backing_fmt = get_option_parameter(param, BLOCK_OPT_BACKING_FMT);
if (backing_file && backing_file->value.s) {
- if (!strcmp(filename, backing_file->value.s)) {
- error_setg(errp, "Error: Trying to create an image with the "
- "same filename as the backing file");
+ if (bdrv_backing_file_loop_check(filename, fmt,
+ backing_file->value.s,
+ backing_fmt->value.s)) {
+ /* There is loop exists in the backing file chain */
goto out;
}
}
-
- backing_fmt = get_option_parameter(param, BLOCK_OPT_BACKING_FMT);
if (backing_fmt && backing_fmt->value.s) {
backing_drv = bdrv_find_format(backing_fmt->value.s);
if (!backing_drv) {
--
1.8.1.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH V4 4/5] block: Add backing file loop check in change_backing_file()
2013-07-22 8:05 [Qemu-devel] [PATCH V4 0/5] Redefine and export backing file loop check Xu Wang
` (2 preceding siblings ...)
2013-07-22 8:05 ` [Qemu-devel] [PATCH V4 3/5] block: Check infinite loop in bdrv_img_create() Xu Wang
@ 2013-07-22 8:05 ` Xu Wang
2013-07-22 8:50 ` Fam Zheng
2013-07-22 8:05 ` [Qemu-devel] [PATCH V4 5/5] block: Add infinite loop check in drive_init() Xu Wang
4 siblings, 1 reply; 9+ messages in thread
From: Xu Wang @ 2013-07-22 8:05 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha, Xu Wang, famz, wdongxu
Backing file loop should be checked before calling change_backing_
file(). If loop appeared, this calling should be stopped and an
error was printed.
Signed-off-by: Xu Wang <cngesaint@gmail.com>
---
block.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/block.c b/block.c
index 9daf63c..d996524 100644
--- a/block.c
+++ b/block.c
@@ -1967,6 +1967,13 @@ int bdrv_change_backing_file(BlockDriverState *bs,
return -EINVAL;
}
+ /* Check if loop exists in backing files chain after changed */
+ if (bdrv_backing_file_loop_check(bs->filename,
+ bs->drv ? bs->drv->format_name : NULL,
+ backing_file, backing_fmt)) {
+ return -EIO;
+ }
+
if (drv->bdrv_change_backing_file != NULL) {
ret = drv->bdrv_change_backing_file(bs, backing_file, backing_fmt);
} else {
--
1.8.1.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH V4 5/5] block: Add infinite loop check in drive_init()
2013-07-22 8:05 [Qemu-devel] [PATCH V4 0/5] Redefine and export backing file loop check Xu Wang
` (3 preceding siblings ...)
2013-07-22 8:05 ` [Qemu-devel] [PATCH V4 4/5] block: Add backing file loop check in change_backing_file() Xu Wang
@ 2013-07-22 8:05 ` Xu Wang
4 siblings, 0 replies; 9+ messages in thread
From: Xu Wang @ 2013-07-22 8:05 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, stefanha, Xu Wang, famz, wdongxu
Backing file should be checked if there is a loop in it during image
boot. Becase if there is loop qemu would no response for a long time
and segment fault occured. So this patch would check backing file
chain if there is loop in it before open image.
Signed-off-by: Xu Wang <cngesaint@gmail.com>
---
blockdev.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/blockdev.c b/blockdev.c
index b3a57e0..590193f 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -695,6 +695,13 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
error_report("warning: disabling copy_on_read on readonly drive");
}
+ /* Add backing file loop check */
+ if (bdrv_backing_file_loop_check(file, drv ? drv->format_name : NULL,
+ NULL, NULL)) {
+ error_report("drive_init: backing file loop check failed!");
+ goto err;
+ }
+
ret = bdrv_open(dinfo->bdrv, file, bs_opts, bdrv_flags, drv);
bs_opts = NULL;
--
1.8.1.4
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH V4 2/5] block: Add WIN32 platform support for backing_file_loop_check()
2013-07-22 8:05 ` [Qemu-devel] [PATCH V4 2/5] block: Add WIN32 platform support for backing_file_loop_check() Xu Wang
@ 2013-07-22 8:40 ` Fam Zheng
0 siblings, 0 replies; 9+ messages in thread
From: Fam Zheng @ 2013-07-22 8:40 UTC (permalink / raw)
To: Xu Wang; +Cc: kwolf, stefanha, wdongxu, qemu-devel
On Mon, 07/22 04:05, Xu Wang wrote:
> Method of get_inode is different between Linux and WIN32 plateform.
> This patch added inode caculate method on Windows plateform so that
> backing file check could work on Windows plateform.
>
> Signed-off-by: Xu Wang <cngesaint@gmail.com>
> ---
> block.c | 160 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 152 insertions(+), 8 deletions(-)
>
> diff --git a/block.c b/block.c
> index aea653d..3e23b67 100644
> --- a/block.c
> +++ b/block.c
> @@ -51,6 +51,10 @@
>
> #define NOT_DONE 0x7fffffff /* used while emulated sync operation in progress */
>
> +#ifdef _WIN32
> +#define MAX_PATH_LEN 8192
> +#endif
> +
> typedef enum {
> BDRV_REQ_COPY_ON_READ = 0x1,
> BDRV_REQ_ZERO_WRITE = 0x2,
> @@ -4433,6 +4437,147 @@ bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie *cookie)
> bs->total_time_ns[cookie->type] += get_clock() - cookie->start_time_ns;
> }
>
> +#ifdef _WIN32
> +static int get_lnk_target_file(const char *lnk_file, char *filepath)
> +{
> + unsigned int flag, offset;
> + unsigned int sflag;
> + char uch;
> + int i = 0;
> +
> + FILE *fd = fopen(lnk_file, "rb");
> + if (!fd) {
> + error_report("Open file %s failed.", lnk_file);
> + return -1;
> + }
> +
> + /* Check if the file is shortcuts by reading first 4 bytes if it's 0x4c */
> + if (fread(&flag, 4, 1, fd) != 1) {
> + error_report("Read 0x4c field of %s failed.", lnk_file);
> + goto err;
> + }
> +
> + if (flag != 0x4c) {
> + error_report("%s is not a lnk file.", lnk_file);
> + goto err;
> + }
> +
> + /* Parse flags on offset 0x14 */
> + if (fseek(fd, 0x14, SEEK_SET)) {
> + error_report("Seek flags of %s failed.", lnk_file);
> + goto err;
> + }
> +
> + if (fread(&flag, 4, 1, fd) != 1) {
> + error_report("Read flags of %s failed.", lnk_file);
> + goto err;
> + }
> +
> + /* seek to the length of item ID list */
> + if (fseek(fd, 0x4c, SEEK_SET)) {
> + error_report("Seek %s length of item ID list failed.", lnk_file);
> + goto err;
> + }
> +
> + /* Parse Shell item ID list */
> + if (flag & 0x01) {
> + /* Get the length of SHITEMID */
> + if (fread(&sflag, 2, 1, fd) != 1) {
> + error_report("Read %s length of SHITEMID failed.", lnk_file);
> + goto err;
> + }
> +
> + /* Skip SHITEMID */
> + if (fseek(fd, sflag, SEEK_CUR)) {
> + error_report("Skip SHITEMID of %s failed.", lnk_file);
> + goto err;
> + }
> + }
> +
> + offset = ftell(fd);
> + if (offset == -1) {
> + error_report("ftell() of %s failed.", lnk_file);
> + goto err;
> + }
> + /* Read offset of local path */
> + if (fseek(fd, offset + 0x10, SEEK_SET)) {
> + error_report("Seek offset of %s failed.", lnk_file);
> + goto err;
> + }
> + if (fread(&flag, 4, 1, fd) != 1) {
> + error_report("Read offset of %s failed.", lnk_file);
> + goto err;
> + }
> + /* Seek to the start of target file path */
> + if (fseek(fd, flag + offset, SEEK_SET)) {
> + error_report("Seek target file path of %s failed.", lnk_file);
> + goto err;
> + }
> +
> + do {
> + if (fread(&uch, 1, 1, fd) != 1) {
> + error_report("Read target path of %s failed.", lnk_file);
> + goto err;
> + }
> + filepath[i++] = uch;
> + } while ((i <= MAX_PATH_LEN) && (uch != '\0'));
> +
> + fclose(fd);
> + return 0;
> +
> +err:
> + fclose(fd);
> + return -1;
> +}
> +#endif
> +
> +static long get_inode(const char *filename)
> +{
> + #ifdef _WIN32
> + char pbuf[MAX_PATH_LEN], *p;
> + long inode;
> + struct stat sbuf;
> + char path[MAX_PATH_LEN];
> + int len;
> +
> + /* If filename contains .lnk, it's a shortcuts. Target file
> + * need to be parsed.
> + */
> + len = strlen(filename);
> + if ((len > 4) && (strcmp(filename + len - 4, ".lnk"))) {
Parenthesis unnecessary for (len > 4), and I think you want
&& (strcmp() == 0)
> + if (get_lnk_target_file(filename, path)) {
> + error_report("Parse .lnk file %s failed.", filename);
> + return -1;
> + }
> + } else {
> + memcpy(path, filename, sizeof(filename));
> + }
> +
> + if (stat(path, &sbuf) == -1) {
> + error_report("get file %s stat error.", path);
> + return -1;
> + }
> + if (GetFullPathName(path, MAX_PATH_LEN, pbuf, &p) != 0) {
> + inode = 11003;
> + for (p = pbuf; *p != '\0'; p++) {
> + inode = inode * 31 + *(unsigned char *)p;
> + }
> + return (inode * 911) & 0x7FFF;
> + }
> +
> + return -1;
> + #else
> + struct stat sbuf;
> +
> + if (stat(filename, &sbuf) == -1) {
> + error_report("get file %s stat error.", filename);
> + return -1;
> + }
> +
> + return sbuf.st_ino;
> + #endif
> +}
> +
> static gboolean str_equal_func(gconstpointer a, gconstpointer b)
> {
> return strcmp(a, b) == 0;
> @@ -4456,7 +4601,6 @@ bool bdrv_backing_file_loop_check(const char *filename, const char *fmt,
> GHashTable *inodes;
> BlockDriverState *bs;
> BlockDriver *drv;
> - struct stat sbuf;
> long inode = 0;
> int ret;
> char fbuf[1024];
> @@ -4468,11 +4612,11 @@ bool bdrv_backing_file_loop_check(const char *filename, const char *fmt,
> if (access(filename, F_OK)) {
> inode = -1;
> } else {
> - if (stat(filename, &sbuf) == -1) {
> - error_report("Get file %s stat failed.", filename);
> + inode = get_inode(filename);
> + if (inode == -1) {
> + error_report("Get file %s inode failed.", filename);
> goto err;
> }
> - inode = (long)sbuf.st_ino;
> }
>
> filename = backing_file;
> @@ -4481,11 +4625,11 @@ bool bdrv_backing_file_loop_check(const char *filename, const char *fmt,
> }
>
> while (filename && (filename[0] != '\0')) {
> - if (stat(filename, &sbuf) == -1) {
> - error_report("Get file %s stat failed.", filename);
> - goto err;
> + inode = get_inode(filename);
> + if (inode == -1) {
> + error_report("Get file %s inode failed.", filename);
> + goto err;
> }
> - inode = (long)sbuf.st_ino;
>
> if (g_hash_table_lookup_extended(inodes, &inode, NULL, NULL)) {
> error_report("Backing file '%s' creates an infinite loop.",
> --
> 1.8.1.4
>
--
Fam
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH V4 3/5] block: Check infinite loop in bdrv_img_create()
2013-07-22 8:05 ` [Qemu-devel] [PATCH V4 3/5] block: Check infinite loop in bdrv_img_create() Xu Wang
@ 2013-07-22 8:44 ` Fam Zheng
0 siblings, 0 replies; 9+ messages in thread
From: Fam Zheng @ 2013-07-22 8:44 UTC (permalink / raw)
To: Xu Wang; +Cc: kwolf, stefanha, wdongxu, qemu-devel
On Mon, 07/22 04:05, Xu Wang wrote:
> Backing file loop should be checked before qemu-img create command
> execution. If loop was found, qemu-img create should be stopped and
> an error was printed.
>
> Signed-off-by: Xu Wang <cngesaint@gmail.com>
> ---
> block.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/block.c b/block.c
> index 3e23b67..9daf63c 100644
> --- a/block.c
> +++ b/block.c
> @@ -4735,15 +4735,15 @@ void bdrv_img_create(const char *filename, const char *fmt,
> }
>
> backing_file = get_option_parameter(param, BLOCK_OPT_BACKING_FILE);
> + backing_fmt = get_option_parameter(param, BLOCK_OPT_BACKING_FMT);
> if (backing_file && backing_file->value.s) {
> - if (!strcmp(filename, backing_file->value.s)) {
> - error_setg(errp, "Error: Trying to create an image with the "
> - "same filename as the backing file");
> + if (bdrv_backing_file_loop_check(filename, fmt,
> + backing_file->value.s,
> + backing_fmt->value.s)) {
> + /* There is loop exists in the backing file chain */
Please add error message here as other error paths do.
> goto out;
> }
> }
> -
> - backing_fmt = get_option_parameter(param, BLOCK_OPT_BACKING_FMT);
> if (backing_fmt && backing_fmt->value.s) {
> backing_drv = bdrv_find_format(backing_fmt->value.s);
> if (!backing_drv) {
> --
> 1.8.1.4
>
--
Fam
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH V4 4/5] block: Add backing file loop check in change_backing_file()
2013-07-22 8:05 ` [Qemu-devel] [PATCH V4 4/5] block: Add backing file loop check in change_backing_file() Xu Wang
@ 2013-07-22 8:50 ` Fam Zheng
0 siblings, 0 replies; 9+ messages in thread
From: Fam Zheng @ 2013-07-22 8:50 UTC (permalink / raw)
To: Xu Wang; +Cc: kwolf, stefanha, wdongxu, qemu-devel
On Mon, 07/22 04:05, Xu Wang wrote:
> Backing file loop should be checked before calling change_backing_
> file(). If loop appeared, this calling should be stopped and an
> error was printed.
>
> Signed-off-by: Xu Wang <cngesaint@gmail.com>
> ---
> block.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/block.c b/block.c
> index 9daf63c..d996524 100644
> --- a/block.c
> +++ b/block.c
> @@ -1967,6 +1967,13 @@ int bdrv_change_backing_file(BlockDriverState *bs,
> return -EINVAL;
> }
>
> + /* Check if loop exists in backing files chain after changed */
> + if (bdrv_backing_file_loop_check(bs->filename,
> + bs->drv ? bs->drv->format_name : NULL,
> + backing_file, backing_fmt)) {
I'm wondering if it will be helpful to add a concrete error message
here, e.g. for qemu-img rebase, so it will be easier for user to know
it fails because of backing loop.
> + return -EIO;
> + }
> +
> if (drv->bdrv_change_backing_file != NULL) {
> ret = drv->bdrv_change_backing_file(bs, backing_file, backing_fmt);
> } else {
> --
> 1.8.1.4
>
--
Fam
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-07-22 8:50 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-22 8:05 [Qemu-devel] [PATCH V4 0/5] Redefine and export backing file loop check Xu Wang
2013-07-22 8:05 ` [Qemu-devel] [PATCH V4 1/5] block/qemu-img: Refine and export infinite loop checking in collect_image_info_list() Xu Wang
2013-07-22 8:05 ` [Qemu-devel] [PATCH V4 2/5] block: Add WIN32 platform support for backing_file_loop_check() Xu Wang
2013-07-22 8:40 ` Fam Zheng
2013-07-22 8:05 ` [Qemu-devel] [PATCH V4 3/5] block: Check infinite loop in bdrv_img_create() Xu Wang
2013-07-22 8:44 ` Fam Zheng
2013-07-22 8:05 ` [Qemu-devel] [PATCH V4 4/5] block: Add backing file loop check in change_backing_file() Xu Wang
2013-07-22 8:50 ` Fam Zheng
2013-07-22 8:05 ` [Qemu-devel] [PATCH V4 5/5] block: Add infinite loop check in drive_init() Xu Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).