From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46674) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Yq33H-0001JZ-65 for qemu-devel@nongnu.org; Wed, 06 May 2015 13:24:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Yq33G-0003Kg-3a for qemu-devel@nongnu.org; Wed, 06 May 2015 13:24:07 -0400 Message-ID: <554A4E2D.4050300@redhat.com> Date: Wed, 06 May 2015 19:23:57 +0200 From: Max Reitz MIME-Version: 1.0 References: <1426791801-9042-1-git-send-email-mreitz@redhat.com> <20150505094606.GB29818@stefanha-thinkpad.redhat.com> <554A1160.7020200@redhat.com> <554A3395.7050509@redhat.com> <554A3D7B.3040603@redhat.com> <554A3F37.9080704@redhat.com> <554A4349.10301@redhat.com> <554A459B.1060505@redhat.com> In-Reply-To: <554A459B.1060505@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [Qemu-block] [PATCH v2 0/3] block: Warn about usage of growing formats over non-growable protocols List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , Stefan Hajnoczi Cc: Stefan Hajnoczi , qemu-devel@nongnu.org, qemu-block@nongnu.org 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