Re: [Qemu-devel] [PATCH V9 3/4] raw-posix: Add full image preallocation option

Re: [Qemu-devel] [PATCH V9 3/4] raw-posix: Add full image preallocation option

[Hu Tao]

On Thu, May 29, 2014 at 08:55:59PM +0200, Peter Lieven wrote:
> Am 29.05.2014 03:55, schrieb Hu Tao:
> > On Wed, May 28, 2014 at 07:36:26AM +0200, Markus Armbruster wrote:
> >> Peter Lieven <pl@kamp.de> writes:
> >>
> >>> Am 27.05.2014 um 21:24 schrieb Eric Blake <eblake@redhat.com>:
> >>>
> >>>> On 05/27/2014 02:22 AM, Chen Fan wrote:
> >>>>> From: Hu Tao <hutao@cn.fujitsu.com>
> >>>>>
> >>>>> This patch adds a new option preallocation for raw format, and implements
> >>>>> full preallocation by writing zeros to disk.
> >>>>>
> >>>>> The metadata option is changed to use posix_fallocate() to ensure
> >>>>> subsquent writes to image file won't fail because of lack of disk space.
> >>>>>
> >>>>> The purpose is to ensure disk space for image file. In cases
> >>>>> posix_fallocate() is supported, metadata option can be used, otherwise
> >>>>> (posix_fallocate() is not supported by filesystem, or in case of thin
> >>>>> provisioning), full option has to be used. User has to choose the proper
> >>>>> way to use.
> >>>>>
> >>>>> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> >>>>> ---
> >>>>> block/raw-posix.c | 61
> >>>>> ++++++++++++++++++++++++++++++++++++++++++++++++-------
> >>>>> 1 file changed, 54 insertions(+), 7 deletions(-)
> >>>>>
> >>>>> @@ -1254,6 +1255,18 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
> >>>>>         if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
> >>>>>             total_size = (options->value.n + BDRV_SECTOR_SIZE) &
> >>>>>                 BDRV_SECTOR_MASK;
> >>>>> +        } else if (!strcmp(options->name, BLOCK_OPT_PREALLOC)) {
> >>>>> +            if (!options->value.s || !strcmp(options->value.s, "off")) {
> >>>>> +                prealloc = PREALLOC_MODE_OFF;
> >>>>> +            } else if (!strcmp(options->value.s, "metadata")) {
> >>>>> +                prealloc = PREALLOC_MODE_METADATA;
> >>>>> +            } else if (!strcmp(options->value.s, "full")) {
> >>>>> +                prealloc = PREALLOC_MODE_FULL;
> >>>>> +            } else {
> >>>> Instead of open-coding this conversion from string to enum, can you
> >>>> reuse Peter's pending patch?  (Hmm, that means Peter needs to export it
> >>>> to somewhere public, not just blockdev.c)
> >>>>
> >>>> https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03772.html
> >>> I don't mind if this function is made public. You can send a patch for it.
> >>> Please don't forget to remove the static declaration. You might
> >>> also want to remove the inline. This was just to avoid an unused
> >>> function error. I don't know if blockdev.c is still the right
> >>> place for this function then.
> >> Probably not.  I'd rename to qapi_enum_parse(), and move it to qapi/.
> >> If no existing file fits, create a new one, say qapi/qapi-util.c.
> >
> > Thanks. I'll use the function in next version.
> 
> Will you introduce a patch to move it to QAPI or shall I?

I will include the patch in next version of this series, but if you'd
like I can send a seperate patch.

Thanks.

Re: [Qemu-devel] [PATCH V9 3/4] raw-posix: Add full image preallocation option

[Peter Lieven]

On 05.06.2014 07:39, Hu Tao wrote:
> On Thu, May 29, 2014 at 08:55:59PM +0200, Peter Lieven wrote:
>> Am 29.05.2014 03:55, schrieb Hu Tao:
>>> On Wed, May 28, 2014 at 07:36:26AM +0200, Markus Armbruster wrote:
>>>> Peter Lieven <pl@kamp.de> writes:
>>>>
>>>>> Am 27.05.2014 um 21:24 schrieb Eric Blake <eblake@redhat.com>:
>>>>>
>>>>>> On 05/27/2014 02:22 AM, Chen Fan wrote:
>>>>>>> From: Hu Tao <hutao@cn.fujitsu.com>
>>>>>>>
>>>>>>> This patch adds a new option preallocation for raw format, and implements
>>>>>>> full preallocation by writing zeros to disk.
>>>>>>>
>>>>>>> The metadata option is changed to use posix_fallocate() to ensure
>>>>>>> subsquent writes to image file won't fail because of lack of disk space.
>>>>>>>
>>>>>>> The purpose is to ensure disk space for image file. In cases
>>>>>>> posix_fallocate() is supported, metadata option can be used, otherwise
>>>>>>> (posix_fallocate() is not supported by filesystem, or in case of thin
>>>>>>> provisioning), full option has to be used. User has to choose the proper
>>>>>>> way to use.
>>>>>>>
>>>>>>> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
>>>>>>> ---
>>>>>>> block/raw-posix.c | 61
>>>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++-------
>>>>>>> 1 file changed, 54 insertions(+), 7 deletions(-)
>>>>>>>
>>>>>>> @@ -1254,6 +1255,18 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
>>>>>>>          if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
>>>>>>>              total_size = (options->value.n + BDRV_SECTOR_SIZE) &
>>>>>>>                  BDRV_SECTOR_MASK;
>>>>>>> +        } else if (!strcmp(options->name, BLOCK_OPT_PREALLOC)) {
>>>>>>> +            if (!options->value.s || !strcmp(options->value.s, "off")) {
>>>>>>> +                prealloc = PREALLOC_MODE_OFF;
>>>>>>> +            } else if (!strcmp(options->value.s, "metadata")) {
>>>>>>> +                prealloc = PREALLOC_MODE_METADATA;
>>>>>>> +            } else if (!strcmp(options->value.s, "full")) {
>>>>>>> +                prealloc = PREALLOC_MODE_FULL;
>>>>>>> +            } else {
>>>>>> Instead of open-coding this conversion from string to enum, can you
>>>>>> reuse Peter's pending patch?  (Hmm, that means Peter needs to export it
>>>>>> to somewhere public, not just blockdev.c)
>>>>>>
>>>>>> https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03772.html
>>>>> I don't mind if this function is made public. You can send a patch for it.
>>>>> Please don't forget to remove the static declaration. You might
>>>>> also want to remove the inline. This was just to avoid an unused
>>>>> function error. I don't know if blockdev.c is still the right
>>>>> place for this function then.
>>>> Probably not.  I'd rename to qapi_enum_parse(), and move it to qapi/.
>>>> If no existing file fits, create a new one, say qapi/qapi-util.c.
>>> Thanks. I'll use the function in next version.
>> Will you introduce a patch to move it to QAPI or shall I?
> I will include the patch in next version of this series, but if you'd
> like I can send a seperate patch.

I would include the patch in the series if the series depends on this patch.

Peter

>
> Thanks.
>


-- 

Mit freundlichen Grüßen

Peter Lieven

...........................................................

   KAMP Netzwerkdienste GmbH
   Vestische Str. 89-91 | 46117 Oberhausen
   Tel: +49 (0) 208.89 402-50 | Fax: +49 (0) 208.89 402-40
   pl@kamp.de | http://www.kamp.de

   Geschäftsführer: Heiner Lante | Michael Lante
   Amtsgericht Duisburg | HRB Nr. 12154
   USt-Id-Nr.: DE 120607556

...........................................................

Re: [Qemu-devel] [PATCH V9 3/4] raw-posix: Add full image preallocation option

[Hu Tao]

On Thu, Jun 05, 2014 at 07:48:12AM +0200, Peter Lieven wrote:
> On 05.06.2014 07:39, Hu Tao wrote:
> >On Thu, May 29, 2014 at 08:55:59PM +0200, Peter Lieven wrote:
> >>Am 29.05.2014 03:55, schrieb Hu Tao:
> >>>On Wed, May 28, 2014 at 07:36:26AM +0200, Markus Armbruster wrote:
> >>>>Peter Lieven <pl@kamp.de> writes:
> >>>>
> >>>>>Am 27.05.2014 um 21:24 schrieb Eric Blake <eblake@redhat.com>:
> >>>>>
> >>>>>>On 05/27/2014 02:22 AM, Chen Fan wrote:
> >>>>>>>From: Hu Tao <hutao@cn.fujitsu.com>
> >>>>>>>
> >>>>>>>This patch adds a new option preallocation for raw format, and implements
> >>>>>>>full preallocation by writing zeros to disk.
> >>>>>>>
> >>>>>>>The metadata option is changed to use posix_fallocate() to ensure
> >>>>>>>subsquent writes to image file won't fail because of lack of disk space.
> >>>>>>>
> >>>>>>>The purpose is to ensure disk space for image file. In cases
> >>>>>>>posix_fallocate() is supported, metadata option can be used, otherwise
> >>>>>>>(posix_fallocate() is not supported by filesystem, or in case of thin
> >>>>>>>provisioning), full option has to be used. User has to choose the proper
> >>>>>>>way to use.
> >>>>>>>
> >>>>>>>Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> >>>>>>>---
> >>>>>>>block/raw-posix.c | 61
> >>>>>>>++++++++++++++++++++++++++++++++++++++++++++++++-------
> >>>>>>>1 file changed, 54 insertions(+), 7 deletions(-)
> >>>>>>>
> >>>>>>>@@ -1254,6 +1255,18 @@ static int raw_create(const char *filename, QEMUOptionParameter *options,
> >>>>>>>         if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
> >>>>>>>             total_size = (options->value.n + BDRV_SECTOR_SIZE) &
> >>>>>>>                 BDRV_SECTOR_MASK;
> >>>>>>>+        } else if (!strcmp(options->name, BLOCK_OPT_PREALLOC)) {
> >>>>>>>+            if (!options->value.s || !strcmp(options->value.s, "off")) {
> >>>>>>>+                prealloc = PREALLOC_MODE_OFF;
> >>>>>>>+            } else if (!strcmp(options->value.s, "metadata")) {
> >>>>>>>+                prealloc = PREALLOC_MODE_METADATA;
> >>>>>>>+            } else if (!strcmp(options->value.s, "full")) {
> >>>>>>>+                prealloc = PREALLOC_MODE_FULL;
> >>>>>>>+            } else {
> >>>>>>Instead of open-coding this conversion from string to enum, can you
> >>>>>>reuse Peter's pending patch?  (Hmm, that means Peter needs to export it
> >>>>>>to somewhere public, not just blockdev.c)
> >>>>>>
> >>>>>>https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg03772.html
> >>>>>I don't mind if this function is made public. You can send a patch for it.
> >>>>>Please don't forget to remove the static declaration. You might
> >>>>>also want to remove the inline. This was just to avoid an unused
> >>>>>function error. I don't know if blockdev.c is still the right
> >>>>>place for this function then.
> >>>>Probably not.  I'd rename to qapi_enum_parse(), and move it to qapi/.
> >>>>If no existing file fits, create a new one, say qapi/qapi-util.c.
> >>>Thanks. I'll use the function in next version.
> >>Will you introduce a patch to move it to QAPI or shall I?
> >I will include the patch in next version of this series, but if you'd
> >like I can send a seperate patch.
> 
> I would include the patch in the series if the series depends on this patch.

OK.