All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Max Reitz <mreitz@redhat.com>
Cc: qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 0/4] block: Drop BDS.filename
Date: Tue, 3 Feb 2015 15:40:58 +0100	[thread overview]
Message-ID: <20150203144058.GG4488@noname.redhat.com> (raw)
In-Reply-To: <54D0D1B8.4000500@redhat.com>

Am 03.02.2015 um 14:48 hat Max Reitz geschrieben:
> On 2015-02-03 at 04:32, Kevin Wolf wrote:
> >Am 24.09.2014 um 21:48 hat Max Reitz geschrieben:
> >>The BDS filename field is generally only used when opening disk images
> >>or emitting error or warning messages, the only exception to this rule
> >>is the map command of qemu-img. However, using exact_filename there
> >>instead should not be a problem. Therefore, we can drop the filename
> >>field from the BlockDriverState and use a function instead which builds
> >>the filename from scratch when called.
> >>
> >>This is slower than reading a static char array but the problem of that
> >>static array is that it may become obsolete due to changes in any
> >>BlockDriverState or in the BDS graph. Using a function which rebuilds
> >>the filename every time it is called resolves this problem.
> >>
> >>The disadvantage of worse performance is negligible, on the other hand.
> >>After patch 2 of this series, which replaces some queries of
> >>BDS.filename by reads from somewhere else (mostly BDS.exact_filename),
> >>the filename field is only used when a disk image is opened or some
> >>message should be emitted, both of which cases do not suffer from the
> >>performance hit.
> >Surprisingly (or not), this one needs rebasing.
> 
> Well...
> 
> >I tried it and it doesn't look too hard, but it's a little bit more than
> >what I'm comfortable with doing while applying a series.
> 
> I admire your courage, but I'm not sure whether this series is ready
> for being applied at all. First we (or I) will have to look into how
> users like libvirt which identify a BDS based on the filename can
> break from applying this series.

Well, I haven't reviewed it, so I can't tell. It didn't have a
(Self-)NACK and it's still on your list of to-be-merged patches, so I
took a look.  You're talking about courage - but I just wasn't
courageous enough yet to attack your larger series... ;-)

Kevin

      reply	other threads:[~2015-02-03 14:41 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-24 19:48 [Qemu-devel] [PATCH 0/4] block: Drop BDS.filename Max Reitz
2014-09-24 19:48 ` [Qemu-devel] [PATCH 1/4] block: Change bdrv_get_encrypted_filename() Max Reitz
2014-09-24 19:48 ` [Qemu-devel] [PATCH 2/4] block: Avoid BlockDriverState.filename Max Reitz
2014-09-24 19:48 ` [Qemu-devel] [PATCH 3/4] block: Add bdrv_filename() Max Reitz
2014-09-24 19:48 ` [Qemu-devel] [PATCH 4/4] block: Drop BlockDriverState.filename Max Reitz
2015-02-03  9:32 ` [Qemu-devel] [PATCH 0/4] block: Drop BDS.filename Kevin Wolf
2015-02-03 13:48   ` Max Reitz
2015-02-03 14:40     ` Kevin Wolf [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=20150203144058.GG4488@noname.redhat.com \
    --to=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --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.