All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Peter Lieven <pl@kamp.de>
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: Mon, 23 Jun 2014 15:05:07 -0600	[thread overview]
Message-ID: <53A89683.4030707@redhat.com> (raw)
In-Reply-To: <439D6F63-2D86-4ABC-9796-4298179201AB@kamp.de>

[-- Attachment #1: Type: text/plain, Size: 2095 bytes --]

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).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

  reply	other threads:[~2014-06-23 21:05 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 [this message]
2014-06-24  8:53       ` Peter Lieven

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=53A89683.4030707@redhat.com \
    --to=eblake@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=pl@kamp.de \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.