All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v5 1/1] block: improve error handling in raw_open
@ 2016-10-11 14:12 Halil Pasic
  2016-10-14 15:59 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
  0 siblings, 1 reply; 4+ messages in thread
From: Halil Pasic @ 2016-10-11 14:12 UTC (permalink / raw)
  To: qemu-block; +Cc: Halil Pasic, Kevin Wolf, Cornelia Huck, qemu-devel, Max Reitz

Make raw_open for POSIX more consistent in handling errors by setting
the error object also when qemu_open fails. The error object was set
generally set in case of errors, but I guess this case was overlooked.
Do the same for win32.

Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Reviewed-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
Tested-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> (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 trying
to hotplug yet another SCSI disk via libvirt we end up with no adequate
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.

The win32 part was not tested, and the sole reason I touched it is
to not introduce unnecessary divergence.

v4 -> v5:
* fix qemu-iotests by adding the filename to the message
v3 -> v4:
* rebased on current master
v2 -> v3:
* first save errno then error_setg_errno
v1 -> v2:
* fixed win32 by the correct error_setg_*
* use the original errno consequently
---
 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 166e9d1..f481e57 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -443,6 +443,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
     fd = qemu_open(filename, s->open_flags, 0644);
     if (fd < 0) {
         ret = -errno;
+        error_setg_errno(errp, errno, "Could not open '%s'", filename);
         if (ret == -EROFS) {
             ret = -EACCES;
         }
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 734bb10..800fabd 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -373,6 +373,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
     if (s->hfile == INVALID_HANDLE_VALUE) {
         int err = GetLastError();
 
+        error_setg_win32(errp, err, "Could not open '%s'", filename);
         if (err == ERROR_ACCESS_DENIED) {
             ret = -EACCES;
         } else {
-- 
2.8.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [Qemu-block] [PATCH v5 1/1] block: improve error handling in raw_open
  2016-10-11 14:12 [Qemu-devel] [PATCH v5 1/1] block: improve error handling in raw_open Halil Pasic
@ 2016-10-14 15:59 ` Stefan Hajnoczi
  2016-10-14 16:27   ` Halil Pasic
  2016-10-17 10:19   ` Kevin Wolf
  0 siblings, 2 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2016-10-14 15:59 UTC (permalink / raw)
  To: Halil Pasic; +Cc: qemu-block, Kevin Wolf, Cornelia Huck, qemu-devel, Max Reitz

[-- Attachment #1: Type: text/plain, Size: 1641 bytes --]

On Tue, Oct 11, 2016 at 04:12:35PM +0200, 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 set
> generally set in case of errors, but I guess this case was overlooked.
> Do the same for win32.
> 
> Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> Reviewed-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
> Tested-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> (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 trying
> to hotplug yet another SCSI disk via libvirt we end up with no adequate
> 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.
> 
> The win32 part was not tested, and the sole reason I touched it is
> to not introduce unnecessary divergence.
> 
> v4 -> v5:
> * fix qemu-iotests by adding the filename to the message

This patch doesn't modify any iotests golden master files.  Does this
mean the iotests output is unchanged?

> v3 -> v4:
> * rebased on current master
> v2 -> v3:
> * first save errno then error_setg_errno
> v1 -> v2:
> * fixed win32 by the correct error_setg_*
> * use the original errno consequently
> ---
>  block/raw-posix.c | 1 +
>  block/raw-win32.c | 1 +
>  2 files changed, 2 insertions(+)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [Qemu-block] [PATCH v5 1/1] block: improve error handling in raw_open
  2016-10-14 15:59 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
@ 2016-10-14 16:27   ` Halil Pasic
  2016-10-17 10:19   ` Kevin Wolf
  1 sibling, 0 replies; 4+ messages in thread
From: Halil Pasic @ 2016-10-14 16:27 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: qemu-block, Kevin Wolf, Cornelia Huck, qemu-devel, Max Reitz

[-- Attachment #1: Type: text/plain, Size: 1619 bytes --]



On 10/14/2016 05:59 PM, Stefan Hajnoczi wrote:
> On Tue, Oct 11, 2016 at 04:12:35PM +0200, 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 set
>> > generally set in case of errors, but I guess this case was overlooked.
>> > Do the same for win32.
>> > 
>> > Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
>> > Reviewed-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
>> > Tested-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> (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 trying
>> > to hotplug yet another SCSI disk via libvirt we end up with no adequate
>> > 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.
>> > 
>> > The win32 part was not tested, and the sole reason I touched it is
>> > to not introduce unnecessary divergence.
>> > 
>> > v4 -> v5:
>> > * fix qemu-iotests by adding the filename to the message
> This patch doesn't modify any iotests golden master files.  Does this
> mean the iotests output is unchanged?
> 

Exactly. Instead of modifying the golden masters I choose to modify
the message generated by me and make it more verbose by appending
the filename in question.

Thanks for the review!

Halil


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [Qemu-block] [PATCH v5 1/1] block: improve error handling in raw_open
  2016-10-14 15:59 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
  2016-10-14 16:27   ` Halil Pasic
@ 2016-10-17 10:19   ` Kevin Wolf
  1 sibling, 0 replies; 4+ messages in thread
From: Kevin Wolf @ 2016-10-17 10:19 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: Halil Pasic, qemu-block, Cornelia Huck, qemu-devel, Max Reitz

[-- Attachment #1: Type: text/plain, Size: 1831 bytes --]

Am 14.10.2016 um 17:59 hat Stefan Hajnoczi geschrieben:
> On Tue, Oct 11, 2016 at 04:12:35PM +0200, 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 set
> > generally set in case of errors, but I guess this case was overlooked.
> > Do the same for win32.
> > 
> > Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> > Reviewed-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
> > Tested-by: Marc Hartmayer <mhartmay@linux.vnet.ibm.com> (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 trying
> > to hotplug yet another SCSI disk via libvirt we end up with no adequate
> > 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.
> > 
> > The win32 part was not tested, and the sole reason I touched it is
> > to not introduce unnecessary divergence.
> > 
> > v4 -> v5:
> > * fix qemu-iotests by adding the filename to the message
> 
> This patch doesn't modify any iotests golden master files.  Does this
> mean the iotests output is unchanged?
> 
> > v3 -> v4:
> > * rebased on current master
> > v2 -> v3:
> > * first save errno then error_setg_errno
> > v1 -> v2:
> > * fixed win32 by the correct error_setg_*
> > * use the original errno consequently
> > ---
> >  block/raw-posix.c | 1 +
> >  block/raw-win32.c | 1 +
> >  2 files changed, 2 insertions(+)
> 
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Thanks, applied to the block branch.

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-10-17 10:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-11 14:12 [Qemu-devel] [PATCH v5 1/1] block: improve error handling in raw_open Halil Pasic
2016-10-14 15:59 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2016-10-14 16:27   ` Halil Pasic
2016-10-17 10:19   ` Kevin Wolf

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.