| Message ID | 1343753510-24661-2-git-send-email-wdongxu@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 07/31/2012 10:51 AM, Dong Xu Wang wrote: > In add-cow file format, we will use path_has_protocol and we will read > a NUL-terminated string from image , qed_read_string has done the samething, s/image ,/image,/ s/samething/same thing/ > so make the two functions public, then we will reuse them directly. > > While creating images files, if no size is specified, will use size of backing > file. If no backing file is specified, we will use the size of image file. > > Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> > --- > block.c | 37 +++++++++++++++++++++++++++++++++---- > block.h | 3 +++ > block/qed.c | 29 +---------------------------- > 3 files changed, 37 insertions(+), 32 deletions(-) > > + /* The size for the image must always be specified, with one exception: > + If we are using a backing file, we can obtain the size from there, > + but if not, and we are using an image file, we will obtain the size from it.*/ grammar: The size for the image must be known, either by direct specification, or by reading it from a backing file or image file.
On Tue, Jul 31, 2012 at 5:51 PM, Dong Xu Wang <wdongxu@linux.vnet.ibm.com> wrote: > diff --git a/block.h b/block.h > index c89590d..b523076 100644 > --- a/block.h > +++ b/block.h > @@ -152,6 +152,8 @@ int bdrv_pwrite(BlockDriverState *bs, int64_t offset, > const void *buf, int count); > int bdrv_pwrite_sync(BlockDriverState *bs, int64_t offset, > const void *buf, int count); > +int bdrv_read_string(BlockDriverState *file, uint64_t offset, size_t n, > + char *buf, size_t buflen); I suggest renaming the "file" argument to "bs" like all the other block.h functions. > /** > - * Read a string of known length from the image file > - * > - * @file: Image file > - * @offset: File offset to start of string, in bytes > - * @n: String length in bytes > - * @buf: Destination buffer > - * @buflen: Destination buffer length in bytes > - * @ret: 0 on success, -errno on failure > - * > - * The string is NUL-terminated. > - */ Please keep the doc comment. Stefan
On Wed, Aug 1, 2012 at 9:53 PM, Eric Blake <eblake@redhat.com> wrote: > On 07/31/2012 10:51 AM, Dong Xu Wang wrote: >> In add-cow file format, we will use path_has_protocol and we will read >> a NUL-terminated string from image , qed_read_string has done the samething, > > s/image ,/image,/ > s/samething/same thing/ > >> so make the two functions public, then we will reuse them directly. >> >> While creating images files, if no size is specified, will use size of backing >> file. If no backing file is specified, we will use the size of image file. >> >> Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> >> --- >> block.c | 37 +++++++++++++++++++++++++++++++++---- >> block.h | 3 +++ >> block/qed.c | 29 +---------------------------- >> 3 files changed, 37 insertions(+), 32 deletions(-) >> >> + /* The size for the image must always be specified, with one exception: >> + If we are using a backing file, we can obtain the size from there, >> + but if not, and we are using an image file, we will obtain the size from it.*/ > > grammar: > The size for the image must be known, either by direct specification, or > by reading it from a backing file or image file. > > -- > Eric Blake eblake@redhat.com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org > Nod, Thank you, Eric
On Wed, Aug 1, 2012 at 10:01 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote: > On Tue, Jul 31, 2012 at 5:51 PM, Dong Xu Wang > <wdongxu@linux.vnet.ibm.com> wrote: >> diff --git a/block.h b/block.h >> index c89590d..b523076 100644 >> --- a/block.h >> +++ b/block.h >> @@ -152,6 +152,8 @@ int bdrv_pwrite(BlockDriverState *bs, int64_t offset, >> const void *buf, int count); >> int bdrv_pwrite_sync(BlockDriverState *bs, int64_t offset, >> const void *buf, int count); >> +int bdrv_read_string(BlockDriverState *file, uint64_t offset, size_t n, >> + char *buf, size_t buflen); > > I suggest renaming the "file" argument to "bs" like all the other > block.h functions. > Okay. >> /** >> - * Read a string of known length from the image file >> - * >> - * @file: Image file >> - * @offset: File offset to start of string, in bytes >> - * @n: String length in bytes >> - * @buf: Destination buffer >> - * @buflen: Destination buffer length in bytes >> - * @ret: 0 on success, -errno on failure >> - * >> - * The string is NUL-terminated. >> - */ > > Please keep the doc comment. > > Stefan Okay. thank you. >
diff --git a/block.c b/block.c index b38940b..3b4d27f 100644 --- a/block.c +++ b/block.c @@ -196,7 +196,7 @@ static void bdrv_io_limits_intercept(BlockDriverState *bs, } /* check if the path starts with "<protocol>:" */ -static int path_has_protocol(const char *path) +int path_has_protocol(const char *path) { const char *p; @@ -213,6 +213,21 @@ static int path_has_protocol(const char *path) return *p == ':'; } +int bdrv_read_string(BlockDriverState *file, uint64_t offset, size_t n, + char *buf, size_t buflen) +{ + int ret; + if (n >= buflen) { + return -EINVAL; + } + ret = bdrv_pread(file, offset, buf, n); + if (ret < 0) { + return ret; + } + buf[n] = '\0'; + return 0; +} + int path_is_absolute(const char *path) { #ifdef _WIN32 @@ -3884,7 +3899,7 @@ int bdrv_img_create(const char *filename, const char *fmt, char *options, uint64_t img_size, int flags) { QEMUOptionParameter *param = NULL, *create_options = NULL; - QEMUOptionParameter *backing_fmt, *backing_file, *size; + QEMUOptionParameter *backing_fmt, *backing_file, *size, *image_file; BlockDriverState *bs = NULL; BlockDriver *drv, *proto_drv; BlockDriver *backing_drv = NULL; @@ -3965,9 +3980,11 @@ int bdrv_img_create(const char *filename, const char *fmt, } } - // The size for the image must always be specified, with one exception: - // If we are using a backing file, we can obtain the size from there + /* The size for the image must always be specified, with one exception: + If we are using a backing file, we can obtain the size from there, + but if not, and we are using an image file, we will obtain the size from it.*/ size = get_option_parameter(param, BLOCK_OPT_SIZE); + image_file = get_option_parameter(param, BLOCK_OPT_IMAGE_FILE); if (size && size->value.n == -1) { if (backing_file && backing_file->value.s) { uint64_t size; @@ -3990,6 +4007,18 @@ int bdrv_img_create(const char *filename, const char *fmt, snprintf(buf, sizeof(buf), "%" PRId64, size); set_option_parameter(param, BLOCK_OPT_SIZE, buf); + } else if (image_file && image_file->value.s) { + uint64_t size; + char buf[32]; + bs = bdrv_new(""); + ret = bdrv_file_open(&bs, image_file->value.s, BDRV_O_RDWR); + if (ret < 0) { + error_report("Could not open '%s'", image_file->value.s); + goto out; + } + size = bdrv_getlength(bs); + snprintf(buf, sizeof(buf), "%" PRId64, size); + set_option_parameter(param, BLOCK_OPT_SIZE, buf); } else { error_report("Image creation needs a size parameter"); ret = -EINVAL; diff --git a/block.h b/block.h index c89590d..b523076 100644 --- a/block.h +++ b/block.h @@ -152,6 +152,8 @@ int bdrv_pwrite(BlockDriverState *bs, int64_t offset, const void *buf, int count); int bdrv_pwrite_sync(BlockDriverState *bs, int64_t offset, const void *buf, int count); +int bdrv_read_string(BlockDriverState *file, uint64_t offset, size_t n, + char *buf, size_t buflen); int coroutine_fn bdrv_co_readv(BlockDriverState *bs, int64_t sector_num, int nb_sectors, QEMUIOVector *qiov); int coroutine_fn bdrv_co_copy_on_readv(BlockDriverState *bs, @@ -306,6 +308,7 @@ char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn); char *get_human_readable_size(char *buf, int buf_size, int64_t size); int path_is_absolute(const char *path); +int path_has_protocol(const char *path); void path_combine(char *dest, int dest_size, const char *base_path, const char *filename); diff --git a/block/qed.c b/block/qed.c index 5f3eefa..311c589 100644 --- a/block/qed.c +++ b/block/qed.c @@ -217,33 +217,6 @@ static bool qed_is_image_size_valid(uint64_t image_size, uint32_t cluster_size, } /** - * Read a string of known length from the image file - * - * @file: Image file - * @offset: File offset to start of string, in bytes - * @n: String length in bytes - * @buf: Destination buffer - * @buflen: Destination buffer length in bytes - * @ret: 0 on success, -errno on failure - * - * The string is NUL-terminated. - */ -static int qed_read_string(BlockDriverState *file, uint64_t offset, size_t n, - char *buf, size_t buflen) -{ - int ret; - if (n >= buflen) { - return -EINVAL; - } - ret = bdrv_pread(file, offset, buf, n); - if (ret < 0) { - return ret; - } - buf[n] = '\0'; - return 0; -} - -/** * Allocate new clusters * * @s: QED state @@ -437,7 +410,7 @@ static int bdrv_qed_open(BlockDriverState *bs, int flags) return -EINVAL; } - ret = qed_read_string(bs->file, s->header.backing_filename_offset, + ret = bdrv_read_string(bs->file, s->header.backing_filename_offset, s->header.backing_filename_size, bs->backing_file, sizeof(bs->backing_file)); if (ret < 0) {
In add-cow file format, we will use path_has_protocol and we will read a NUL-terminated string from image , qed_read_string has done the samething, so make the two functions public, then we will reuse them directly. While creating images files, if no size is specified, will use size of backing file. If no backing file is specified, we will use the size of image file. Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> --- block.c | 37 +++++++++++++++++++++++++++++++++---- block.h | 3 +++ block/qed.c | 29 +---------------------------- 3 files changed, 37 insertions(+), 32 deletions(-)