From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46785) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPAnq-00057P-Sq for qemu-devel@nongnu.org; Mon, 18 Jul 2016 11:49:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bPAno-0001Re-S8 for qemu-devel@nongnu.org; Mon, 18 Jul 2016 11:49:53 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:60603) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bPAno-0001RZ-Jj for qemu-devel@nongnu.org; Mon, 18 Jul 2016 11:49:52 -0400 Received: from pps.filterd (m0098396.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.11/8.16.0.11) with SMTP id u6IFnAE3115644 for ; Mon, 18 Jul 2016 11:49:51 -0400 Received: from e06smtp11.uk.ibm.com (e06smtp11.uk.ibm.com [195.75.94.107]) by mx0a-001b2d01.pphosted.com with ESMTP id 24827ewrft-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Mon, 18 Jul 2016 11:49:51 -0400 Received: from localhost by e06smtp11.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 18 Jul 2016 16:49:45 +0100 References: <1468845016-43468-1-git-send-email-pasic@linux.vnet.ibm.com> From: Halil Pasic Date: Mon, 18 Jul 2016 17:48:39 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="nrjEAoQt1qG1oLOueXk6hECqarTMCq7W2" Message-Id: <578CFA57.3080806@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [Qemu-block] [PATCH 1/1] block: improve error handling in raw_open List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz , qemu-block@nongnu.org Cc: Kevin Wolf , Cornelia Huck , qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --nrjEAoQt1qG1oLOueXk6hECqarTMCq7W2 From: Halil Pasic To: Max Reitz , qemu-block@nongnu.org Cc: Kevin Wolf , Cornelia Huck , qemu-devel@nongnu.org Message-ID: <578CFA57.3080806@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [Qemu-block] [PATCH 1/1] block: improve error handling in raw_open References: <1468845016-43468-1-git-send-email-pasic@linux.vnet.ibm.com> In-Reply-To: Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable On 07/18/2016 04:41 PM, Max Reitz wrote: > On 18.07.2016 14:30, Halil Pasic wrote: >> Make raw_open for POSIX more consistent in handling errors by setting >> the error object also when qemu_open fails. The error object was >> generally set in case of errors, but I guess this case was overlooked.= >> Do the same for win32. >> >> Signed-off-by: Halil Pasic >> Reviewed-by: Cornelia Huck >> Reviewed-by: Sascha Silbe >> Tested-by: Marc Hartmayer (POSIX only) >> >> --- >> >> Stumbled upon this (POSIX) while testing VMs with too many SCSI disks = in >> respect to my nofile limit. When open hits the nofile limit while tryi= ng >> to hotplug yet another SCSI disk via libvirt we end up with no adequat= e >> error message (one stating too many files). Sadly this patch in not >> sufficient to fix this problem because drive_new (/qemu/blockdev.c) >> handles errors using error_report_err which is documented as not to be= >> used in QMP context. Do not have a patch for that, because I'm unsure >> whats the best way to deal with it. My guess right now is to make sure= >> we propagate errors at least until reaching code which is called only= >> QMP in context and handle communicating the error to the requester of >> the operation there. Any suggestions or ideas? >> >> The win32 part was not tested, and the sole reason I touched it is >> to not introduce unnecessary divergence. >> --- >> block/raw-posix.c | 1 + >> block/raw-win32.c | 1 + >> 2 files changed, 2 insertions(+) >> >> diff --git a/block/raw-posix.c b/block/raw-posix.c >> index c979ac3..4a7056e 100644 >> --- a/block/raw-posix.c >> +++ b/block/raw-posix.c >> @@ -489,6 +489,7 @@ static int raw_open_common(BlockDriverState *bs, Q= Dict *options, >> if (ret =3D=3D -EROFS) { >> ret =3D -EACCES; >> } >> + error_setg_errno(errp, -ret, "Could not open file"); >=20 > How about putting this above the "if (ret =3D=3D -EROFS)" block? While = other > parts of qemu may want to treat EROFS and EACCES in the same way, I > think it makes sense to distinguish both cases in messages meant for a > human user. >=20 > Max Thanks for the comment! Have no strong opinion here. AFAIU the errno argument is only used to generate a message so there should be no consistency issue, and it would be more consistent with the win32. How about moving both (posix and win32) before the conditional statements readjusting the return value and use errno and err directly? Cheers, Halil >=20 >> goto fail; >> } >> s->fd =3D fd; >> diff --git a/block/raw-win32.c b/block/raw-win32.c >> index 62edb1a..f324f4e 100644 >> --- a/block/raw-win32.c >> +++ b/block/raw-win32.c >> @@ -342,6 +342,7 @@ static int raw_open(BlockDriverState *bs, QDict *o= ptions, int flags, >> } else { >> ret =3D -EINVAL; >> } >> + error_setg_errno(errp, err, "Could not open file"); >> goto fail; >> } >> =20 >> >=20 >=20 --nrjEAoQt1qG1oLOueXk6hECqarTMCq7W2 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.14 (GNU/Linux) iQIcBAEBAgAGBQJXjPqWAAoJEA0vhuyXGx0ABDcP/2SG3SJXBZyVbObL/gv/nVWj 6fmo5If/KfHbPcZ3XBYIPCppX09kj8p+s+HVvA+Fo57Pe8e8iy60MFNYVZj5CxWl aPycz8dpOZwOX3ePEleU6XCBib/MjXTh4jVUQwh74E6RtSVQqrz8IcLKvUE1jaOT jZ+SVVThzQoCVKRKB/j4l0J8kK8Vp97Ln9lW1Ijz5WysMUBBuqG/n5Y37VaTEZuo oTgHaYtMG3eHZp/SgByj0HI6RZFKpRxi8aW728w6tzwXZNNRi+J+tmDeXm2yJ2jh xV6tWFYc+LWm1i9mbV4RMu231Sk9RnUwD6EOxjDhagvsk4bK9b9NlEMxN/WiX+t/ adRr6nzWpi6c5CqY8ohkNZmN9tq2vYsNHyNVyvaDOW5XZix40zqkCkFS3ISAt6G3 20klSm+ML4hvs9EFvEScfKOOYZaY2xfGRZGGwDpON2XslledJRDLVzL+I1FFmzBf pKDVFm5oHK+2DIOM/421c8o4qifThkXjgn1iwnMrMa3h6HH8IJT1Wi0uFLYPomM/ Tza7cqI1M+C0qpwS7RL0RGi+vs9guQ3cOzoQBIp/2Ly4bIIvApJcIRsZCZ3WkVdG 7qEyzb6sBchLc5tkI0kzLAv9HMwZHn6WxJ4onOKkNQ+6+d1hhVPT66vdzZcb57lp vAU9qF6hll87mNG+8Qky =DRPz -----END PGP SIGNATURE----- --nrjEAoQt1qG1oLOueXk6hECqarTMCq7W2--