qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Peter Lieven <pl@kamp.de>
To: Eric Blake <eblake@redhat.com>
Cc: kwolf@redhat.com, pbonzini@redhat.com, stefanha@redhat.com,
	qemu-devel@nongnu.org, ronniesahlberg@gmail.com
Subject: Re: [Qemu-devel] [PATCH] block/nfs: add knob to set readahead
Date: Tue, 24 Jun 2014 10:53:41 +0200	[thread overview]
Message-ID: <53A93C95.9040104@kamp.de> (raw)
In-Reply-To: <53A89683.4030707@redhat.com>

On 23.06.2014 23:05, Eric Blake wrote:
> On 06/23/2014 02:47 PM, Peter Lieven wrote:
>
>>>> +#ifdef LIBNFS_FEATURE_READAHEAD
>>>> +        } else if (!strcmp(qp->p[i].name, "readahead")) {
>>>> +            nfs_set_readahead(client->context, atoi(qp->p[i].value));
>>>> +#endif
>>> I'm not a fan of adding even more special-casing to the file-name URI
>>> without also adding it to the QAPI schema for use in the blockdev-add
>>> command.  Of course, we don't have structured nfs options for
>>> blockdev-add yet, but maybe it's time to start thinking about that
>>> addition before we do this addition.
>> I would like to leave this for a follow-up and I promise to take care of this.
>> If you could give me a pointer of an existing driver that does this as a reference
>> I would be grateful.
> A good example of a recent conversion to structured options would be the
> blkdebug and blkverify backends (look around commit 1bf20b82), or
> proposed addition of new backends (such as archipelego,
> https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg05309.html).
> It may be a bigger task than you anticipate, and certainly won't make
> the 2.1 timeframe, but it's worth doing so if you're up to the task,
> more power to you :)
>
>> For the read ahead parameter its likely not needed as (at least in my use case)
>> the read ahead makes only sense when converting a compressed QCOW2 Template File
>> which lives on an NFS Share to a RAW Device. So I use it with qemu-img.
>> For a Container File that is used as a VM hard drive I would likely not use read ahead
>> as the operating system itself will implement some sort of read ahead already.
> Even if qemu-img is likely to be the only one ever specifying the
> option, it still makes sense to expose it via QMP, as we are gradually
> trying to move qemu-img over to more exclusive use of QMP (or at least
> the underlying functions reached by QMP) rather than ad-hoc code.  For
> example,
> https://lists.gnu.org/archive/html/qemu-devel/2014-06/msg01822.html).
>

Ok, it looks  like a bit of work. I will put it on my list for 2.2.

Thanks for the pointers,
Peter

      reply	other threads:[~2014-06-24  8:54 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-23 14:30 [Qemu-devel] [PATCH] block/nfs: add knob to set readahead Peter Lieven
2014-06-23 15:11 ` Eric Blake
2014-06-23 20:47   ` Peter Lieven
2014-06-23 21:05     ` Eric Blake
2014-06-24  8:53       ` Peter Lieven [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53A93C95.9040104@kamp.de \
    --to=pl@kamp.de \
    --cc=eblake@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=ronniesahlberg@gmail.com \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).