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 <address@hidden> writes:
> > > >
> > > > > Am 27.05.2014 um 21:24 schrieb Eric Blake <address@hidden>:
> > > > >
> > > > > > On 05/27/2014 02:22 AM, Chen Fan wrote:
> > > > > > > From: Hu Tao <address@hidden>
> > > > > > >
> > > > > > > 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 <address@hidden>
> > > > > > > ---
> > > > > > > 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
  address@hidden | http://www.kamp.de

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

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