qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: John Snow <jsnow@redhat.com>
To: Max Reitz <mreitz@redhat.com>, qemu-block@nongnu.org
Cc: kwolf@redhat.com, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v2 0/4] block: allow partial info-block query
Date: Mon, 14 Dec 2015 12:23:37 -0500	[thread overview]
Message-ID: <566EFB19.8020806@redhat.com> (raw)
In-Reply-To: <566EF8D7.3050408@redhat.com>



On 12/14/2015 12:13 PM, Max Reitz wrote:
> On 12.12.2015 02:25, John Snow wrote:
>> Max: Did you have in mind something like this?
> 
> What I had in mind was fixing the whole thing (the test case in 110
> should actually work just fine, because you can easily determine the
> base directory to be used). ;-)
> 

Well, yes... though I figured there was some value in this bit alone --
assuming that the fixing of the filename strategy would take longer. You
understand this problem best AFAICT.

> I've been working on that myself anyway, so yes, regarding not aborting
> block-status if some full backing filename cannot be reconstructed, this
> looks good (by preventing qemu-img from resorting to the relative
> filename when querying the backing chain).
> 

I look forward to a less confusing filename situation. :|

> There is a single thing I don't find quite perfect:
> 
> $ mkdir -p foo
> $ ./qemu-img create -f qcow2 foo/base.qcow2 64M
> $ ./qemu-img create -f qcow2 -b base.qcow2 foo/top.qcow2
> $ ./qemu-img info foo/top.qcow2
> [...]
> backing file: base.qcow2 (actual path: foo/base.qcow2)
> [...]
> 
> $ ./qemu-img info \
>     "json:{'lazy-refcounts':true,'file.filename':'foo/top.qcow2'}"
> [...]
> backing file: base.qcow2
> [...]
> 
> $ cd foo
> $ ../qemu-img info top.qcow2
> [...]
> backing file: base.qcow2
> [...]
> 
> Before this series, there was only one case where the actual path
> information was missing: If it equals the raw relative filename. Now,
> there are two cases: As can be seen in the last invokation, it still
> will not be emitted if actual and raw filename are equal. But as you can
> see in the second invokation, it will be hidden also if it simply cannot
> be determined.
> 

Tch, yes.

> Therefore, if the actual path information is missing, the user cannot
> determine[1] whether this is due to the actual path (relative to the
> CWD) is equal to the (raw) relative path to the overlay; or whether this
> is due to some error in determining the absolute filename.
> 
> [1] You can by testing whether --backing-chain emits an error. But that
> is a bit crude.
> 
> I think we should put a note there if an error occurred while
> determining the absolute backing filename.
> 
> (Doing that is pretty simple:
> if (!info->has_full_backing_filename) {
>     func_fprintf(f, " (cannot determine actual path)");
> } else if (strcmp(...)) {
>     func_fprintf(f, " (actual path: %s)", ...);
> }
> )
> 
> I think that should be an additional patch between 2 and 4.
> 
> Max
> 

Yes, thanks.

>>
>> v2:
>>  - Fix qemu-img from now choking when it gets a partial response.
>>
>> ________________________________________________________________________________
>>
>> For convenience, this branch is available at:
>> https://github.com/jnsnow/qemu.git branch block-allow-partial-query
>> https://github.com/jnsnow/qemu/tree/block-allow-partial-query
>>
>> This version is tagged block-allow-partial-query-v2:
>> https://github.com/jnsnow/qemu/releases/tag/block-allow-partial-query-v2
>>
>> John Snow (4):
>>   block/qapi: do not redundantly print "actual path"
>>   block/qapi: always report full_backing_filename
>>   qemu-img: abort when full_backing_filename not present
>>   block/qapi: allow best-effort query
>>
>>  block/qapi.c               | 18 +++++++++++-------
>>  qemu-img.c                 |  5 ++++-
>>  tests/qemu-iotests/043.out |  2 ++
>>  tests/qemu-iotests/110.out |  5 ++++-
>>  4 files changed, 21 insertions(+), 9 deletions(-)
>>
> 
> 

      reply	other threads:[~2015-12-14 17:23 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-12  1:25 [Qemu-devel] [PATCH v2 0/4] block: allow partial info-block query John Snow
2015-12-12  1:25 ` [Qemu-devel] [PATCH v2 1/4] block/qapi: do not redundantly print "actual path" John Snow
2015-12-14 15:17   ` Max Reitz
2015-12-12  1:25 ` [Qemu-devel] [PATCH v2 2/4] block/qapi: always report full_backing_filename John Snow
2015-12-14 16:36   ` Max Reitz
2015-12-14 16:38     ` John Snow
2015-12-14 16:39       ` Max Reitz
2015-12-12  1:25 ` [Qemu-devel] [PATCH v2 3/4] qemu-img: abort when full_backing_filename not present John Snow
2015-12-14 16:39   ` Max Reitz
2015-12-12  1:25 ` [Qemu-devel] [PATCH v2 4/4] block/qapi: allow best-effort query John Snow
2015-12-14 16:50   ` Max Reitz
2015-12-14 17:13 ` [Qemu-devel] [PATCH v2 0/4] block: allow partial info-block query Max Reitz
2015-12-14 17:23   ` John Snow [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=566EFB19.8020806@redhat.com \
    --to=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /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).