qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Max Reitz <mreitz@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>,
	Stefan Hajnoczi <stefanha@gmail.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>,
	qemu-devel@nongnu.org, qemu-block@nongnu.org
Subject: Re: [Qemu-devel] [Qemu-block] [PATCH v2 0/3] block: Warn about usage of growing formats over non-growable protocols
Date: Wed, 06 May 2015 19:23:57 +0200	[thread overview]
Message-ID: <554A4E2D.4050300@redhat.com> (raw)
In-Reply-To: <554A459B.1060505@redhat.com>

On 06.05.2015 18:47, Paolo Bonzini wrote:
>
> On 06/05/2015 18:37, Max Reitz wrote:
>> Because qcow2 tries to write beyond the end of the file; the NBD client
>> implementation passes that on to the server, and the server simply
>> reports an error (which the NBD client turns into EIO).
> Where?
>
>      qemu_coroutine_yield();
>      *reply = s->reply;
>      if (reply->handle != request->handle) {
>          reply->error = EIO;
>      } else {
>          if (qiov && reply->error == 0) {
>              ret = qemu_co_recvv(s->sock, qiov->iov, qiov->niov,
>                                  offset, request->len);
>              if (ret != request->len) {
>                  reply->error = EIO;
>              }
>          }
>
>          /* Tell the read handler to read another header.  */
>          s->reply.handle = 0;
>      }
>
> It should get into the "else" and then see reply->error != 0.  So the
> guest should see ENOSPC.

The guest sees whatever has been written into reply->error, and that 
field hasn't been written by this function in that case. It has been 
written by nbd_receive_reply() in nbd.c, and that value comes directly 
from the server. In case of qemu-nbd being the server, a write beyond 
the EOF should be caught by blk_check_byte_request() in 
block/block-backend.c, which returns -EIO. So that's where the EIO comes 
from.

I don't know whether this EIO is subsequently converted to ENOSPC 
because of werror=enospc, but considering that 
https://bugzilla.redhat.com/show_bug.cgi?id=1090713 did not override 
werror, it doesn't look like it.

>>> Can you check if virtio-scsi
>>> gives ENOSPC?
>> In which configuration? Using virtio-scsi on top of qcow2 on top of some
>> SCSI passthrough block driver?  Sounds like we ought to make NBD return
>> ENOSPC no matter the fate of this series.
> virtio-scsi on top of qcow2 on top of NBD.  No passthrough: "-device
> scsi-disk" should work.  It seems to me that NBD _should_ be getting
> ENOSPC, unless it's nbd-server that throws away the error.

Well, nbd-server will return whatever it feels like, I don't know about 
that. qemu-nbd on the other hand I do know about, and as written above, 
it will return EIO because of the check performed from blk_write(). So 
that error is sent to the client, where the nbd BDS will return it, too, 
and then qcow2 will propagate it up to virtio-scsi. It looks to me like 
scsi_handle_rw_error() will then call scsi_check_condition(r, 
SENSE_CODE(IO_ERROR)).

(Note that I think werror is only used for block jobs, but not for 
normal guest operations, so EIO coming from a BDS is not converted to 
ENOSPC for guest requests)

I can test these assumptions, if you'd like me to, but that will have to 
wait until Friday. :-)

Max

>> The problem with only warning for a certain non-default configuration is
>> that people who don't know what they are doing are more likely to use
>> the default configuration, so I'd like the warning to appear then.
> That's entirely true.  Perhaps the werror default should be changed from
> enospc to report after all!
>
> Paolo

  reply	other threads:[~2015-05-06 17:24 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-19 19:03 [Qemu-devel] [PATCH v2 0/3] block: Warn about usage of growing formats over non-growable protocols Max Reitz
2015-03-19 19:03 ` [Qemu-devel] [PATCH v2 1/3] iotests: Make nested read in 072 and 089 read-only Max Reitz
2015-03-19 19:23   ` Eric Blake
2015-03-19 19:03 ` [Qemu-devel] [PATCH v2 2/3] block: Introduce BDS.growing Max Reitz
2015-03-19 20:11   ` Eric Blake
2015-03-19 19:03 ` [Qemu-devel] [PATCH v2 3/3] block: Introduce BlockDriver.requires_growing_file Max Reitz
2015-03-19 20:18   ` Eric Blake
2015-05-05  9:46 ` [Qemu-devel] [PATCH v2 0/3] block: Warn about usage of growing formats over non-growable protocols Stefan Hajnoczi
2015-05-06 13:04   ` Max Reitz
2015-05-06 15:30     ` Paolo Bonzini
2015-05-06 16:12       ` [Qemu-devel] [Qemu-block] " Max Reitz
2015-05-06 16:20         ` Paolo Bonzini
2015-05-06 16:37           ` Max Reitz
2015-05-06 16:47             ` Paolo Bonzini
2015-05-06 17:23               ` Max Reitz [this message]
2015-05-07 12:20                 ` Paolo Bonzini
2015-05-07 12:29                   ` Kevin Wolf
2015-05-07 12:47                     ` Paolo Bonzini
2015-05-07 13:20                       ` Kevin Wolf
2015-05-07 13:55                         ` Paolo Bonzini
2015-05-07 14:07                           ` Kevin Wolf
2015-05-07 14:16                             ` Paolo Bonzini
2015-05-07 14:34                               ` Kevin Wolf
2015-05-07 14:50                                 ` Paolo Bonzini
2015-05-08 10:08                                   ` Kevin Wolf
2015-05-08 10:16                                     ` Paolo Bonzini
2015-05-08 10:34                                       ` Kevin Wolf
2015-05-08 11:00                                         ` Paolo Bonzini
2015-05-08 12:58                                     ` Max Reitz

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=554A4E2D.4050300@redhat.com \
    --to=mreitz@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@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).