diff mbox

[v7,13/35] hostmem-file: use whole file size if possible

Message ID 1446455617-129562-14-git-send-email-guangrong.xiao@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Xiao Guangrong Nov. 2, 2015, 9:13 a.m. UTC
Use the whole file size if @size is not specified which is useful
if we want to directly pass a file to guest

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 backends/hostmem-file.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

Comments

Vladimir Sementsov-Ogievskiy Nov. 2, 2015, 5:09 p.m. UTC | #1
On 02.11.2015 12:13, Xiao Guangrong wrote:
> Use the whole file size if @size is not specified which is useful
> if we want to directly pass a file to guest
>
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> ---
>   backends/hostmem-file.c | 22 ++++++++++++++++++----
>   1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> index 9097a57..ea355c1 100644
> --- a/backends/hostmem-file.c
> +++ b/backends/hostmem-file.c
> @@ -38,15 +38,29 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>   {
>       HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
>   
> -    if (!backend->size) {
> -        error_setg(errp, "can't create backend with size 0");
> -        return;
> -    }
>       if (!fb->mem_path) {
>           error_setg(errp, "mem-path property not set");
>           return;
>       }
>   
> +    if (!backend->size) {
> +        Error *local_err = NULL;
> +
> +        /*
> +         * use the whole file size if @size is not specified.
> +         */
> +        backend->size = qemu_file_getlength(fb->mem_path, &local_err);
> +        if (local_err) {
> +            error_propagate(errp, local_err);
> +            return;
> +        }
> +    }
> +
> +    if (!backend->size) {
> +        error_setg(errp, "can't create backend on the file whose size is 0");
> +        return;
> +    }
> +
>       backend->force_prealloc = mem_prealloc;
>       memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
>                                    object_get_canonical_path(OBJECT(backend)),

why not just

+    if (!backend->size) {
+        /*
+         * use the whole file size if @size is not specified.
+         */
+        backend->size = qemu_file_getlength(fb->mem_path, errp);
+        if (*errp) {
+            return;
+        }
+    }


what the purpose of propagating?
Xiao Guangrong Nov. 3, 2015, 2:51 p.m. UTC | #2
On 11/03/2015 01:09 AM, Vladimir Sementsov-Ogievskiy wrote:
> On 02.11.2015 12:13, Xiao Guangrong wrote:
>> Use the whole file size if @size is not specified which is useful
>> if we want to directly pass a file to guest
>>
>> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
>> ---
>>   backends/hostmem-file.c | 22 ++++++++++++++++++----
>>   1 file changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
>> index 9097a57..ea355c1 100644
>> --- a/backends/hostmem-file.c
>> +++ b/backends/hostmem-file.c
>> @@ -38,15 +38,29 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>>   {
>>       HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
>> -    if (!backend->size) {
>> -        error_setg(errp, "can't create backend with size 0");
>> -        return;
>> -    }
>>       if (!fb->mem_path) {
>>           error_setg(errp, "mem-path property not set");
>>           return;
>>       }
>> +    if (!backend->size) {
>> +        Error *local_err = NULL;
>> +
>> +        /*
>> +         * use the whole file size if @size is not specified.
>> +         */
>> +        backend->size = qemu_file_getlength(fb->mem_path, &local_err);
>> +        if (local_err) {
>> +            error_propagate(errp, local_err);
>> +            return;
>> +        }
>> +    }
>> +
>> +    if (!backend->size) {
>> +        error_setg(errp, "can't create backend on the file whose size is 0");
>> +        return;
>> +    }
>> +
>>       backend->force_prealloc = mem_prealloc;
>>       memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
>>                                    object_get_canonical_path(OBJECT(backend)),
>
> why not just

It look like it is a common style used in whole QEMU code.

>
> +    if (!backend->size) {
> +        /*
> +         * use the whole file size if @size is not specified.
> +         */
> +        backend->size = qemu_file_getlength(fb->mem_path, errp);
> +        if (*errp) {
> +            return;
> +        }
> +    }
>
>

But i think your way is better. :)

> what the purpose of propagating?
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index 9097a57..ea355c1 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -38,15 +38,29 @@  file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
 {
     HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
 
-    if (!backend->size) {
-        error_setg(errp, "can't create backend with size 0");
-        return;
-    }
     if (!fb->mem_path) {
         error_setg(errp, "mem-path property not set");
         return;
     }
 
+    if (!backend->size) {
+        Error *local_err = NULL;
+
+        /*
+         * use the whole file size if @size is not specified.
+         */
+        backend->size = qemu_file_getlength(fb->mem_path, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            return;
+        }
+    }
+
+    if (!backend->size) {
+        error_setg(errp, "can't create backend on the file whose size is 0");
+        return;
+    }
+
     backend->force_prealloc = mem_prealloc;
     memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
                                  object_get_canonical_path(OBJECT(backend)),