[Qemu-devel] [6388] Stop VM on ENOSPC error.

[Qemu-devel] [6388] Stop VM on ENOSPC error.

[Anthony Liguori]

Revision: 6388
          http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6388
Author:   aliguori
Date:     2009-01-21 18:59:04 +0000 (Wed, 21 Jan 2009)

Log Message:
-----------
Stop VM on ENOSPC error. (Gleb Natapov)

This version of the patch adds new option "werror" to -drive flag.
Possible values are:

report    - report errors to a guest as IO errors
ignore    - continue as if nothing happened
stop      - stop VM on any error and retry last command on resume
enospc    - stop vm on ENOSPC error and retry last command on resume
            all other errors are reported to a guest.

Default is "report" to maintain current behaviour.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

Modified Paths:
--------------
    trunk/hw/ide.c
    trunk/sysemu.h
    trunk/vl.c

Modified: trunk/hw/ide.c
===================================================================
--- trunk/hw/ide.c	2009-01-21 18:58:51 UTC (rev 6387)
+++ trunk/hw/ide.c	2009-01-21 18:59:04 UTC (rev 6388)
@@ -457,6 +457,8 @@
 #define BM_STATUS_DMAING 0x01
 #define BM_STATUS_ERROR  0x02
 #define BM_STATUS_INT    0x04
+#define BM_STATUS_DMA_RETRY  0x08
+#define BM_STATUS_PIO_RETRY  0x10
 
 #define BM_CMD_START     0x01
 #define BM_CMD_READ      0x08
@@ -488,6 +490,8 @@
     IDEState *ide_if;
     BlockDriverCompletionFunc *dma_cb;
     BlockDriverAIOCB *aiocb;
+    int64_t sector_num;
+    uint32_t nsector;
 } BMDMAState;
 
 typedef struct PCIIDEState {
@@ -498,6 +502,7 @@
 } PCIIDEState;
 
 static void ide_dma_start(IDEState *s, BlockDriverCompletionFunc *dma_cb);
+static void ide_dma_restart(IDEState *s);
 static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret);
 
 static void padstr(char *str, const char *src, int len)
@@ -865,6 +870,28 @@
     ide_set_irq(s);
 }
 
+static int ide_handle_write_error(IDEState *s, int error, int op)
+{
+    BlockInterfaceErrorAction action = drive_get_onerror(s->bs);
+
+    if (action == BLOCK_ERR_IGNORE)
+        return 0;
+
+    if ((error == ENOSPC && action == BLOCK_ERR_STOP_ENOSPC)
+            || action == BLOCK_ERR_STOP_ANY) {
+        s->bmdma->ide_if = s;
+        s->bmdma->status |= op;
+        vm_stop(0);
+    } else {
+        if (op == BM_STATUS_DMA_RETRY)
+            ide_dma_error(s);
+        else
+            ide_rw_error(s);
+    }
+
+    return 1;
+}
+
 /* return 0 if buffer completed */
 static int dma_buf_rw(BMDMAState *bm, int is_write)
 {
@@ -990,9 +1017,10 @@
     if (n > s->req_nb_sectors)
         n = s->req_nb_sectors;
     ret = bdrv_write(s->bs, sector_num, s->io_buffer, n);
+
     if (ret != 0) {
-	ide_rw_error(s);
-	return;
+        if (ide_handle_write_error(s, -ret, BM_STATUS_PIO_RETRY))
+            return;
     }
 
     s->nsector -= n;
@@ -1024,6 +1052,20 @@
     }
 }
 
+static void ide_dma_restart_cb(void *opaque, int running)
+{
+    BMDMAState *bm = opaque;
+    if (!running)
+        return;
+    if (bm->status & BM_STATUS_DMA_RETRY) {
+        bm->status &= ~BM_STATUS_DMA_RETRY;
+        ide_dma_restart(bm->ide_if);
+    } else if (bm->status & BM_STATUS_PIO_RETRY) {
+        bm->status &= ~BM_STATUS_PIO_RETRY;
+        ide_sector_write(bm->ide_if);
+    }
+}
+
 static void ide_write_dma_cb(void *opaque, int ret)
 {
     BMDMAState *bm = opaque;
@@ -1032,8 +1074,8 @@
     int64_t sector_num;
 
     if (ret < 0) {
-	ide_dma_error(s);
-	return;
+        if (ide_handle_write_error(s, -ret,  BM_STATUS_DMA_RETRY))
+            return;
     }
 
     n = s->io_buffer_size >> 9;
@@ -2849,11 +2891,25 @@
     bm->cur_prd_last = 0;
     bm->cur_prd_addr = 0;
     bm->cur_prd_len = 0;
+    bm->sector_num = ide_get_sector(s);
+    bm->nsector = s->nsector;
     if (bm->status & BM_STATUS_DMAING) {
         bm->dma_cb(bm, 0);
     }
 }
 
+static void ide_dma_restart(IDEState *s)
+{
+    BMDMAState *bm = s->bmdma;
+    ide_set_sector(s, bm->sector_num);
+    s->io_buffer_index = 0;
+    s->io_buffer_size = 0;
+    s->nsector = bm->nsector;
+    bm->cur_addr = bm->addr;
+    bm->dma_cb = ide_write_dma_cb;
+    ide_dma_start(s, bm->dma_cb);
+}
+
 static void ide_dma_cancel(BMDMAState *bm)
 {
     if (bm->status & BM_STATUS_DMAING) {
@@ -3043,6 +3099,7 @@
         d->ide_if[2 * i].bmdma = bm;
         d->ide_if[2 * i + 1].bmdma = bm;
         bm->pci_dev = (PCIIDEState *)pci_dev;
+        qemu_add_vm_change_state_handler(ide_dma_restart_cb, bm);
 
         register_ioport_write(addr, 1, 1, bmdma_cmd_writeb, bm);
 
@@ -3068,9 +3125,14 @@
 
     for(i = 0; i < 2; i++) {
         BMDMAState *bm = &d->bmdma[i];
+        uint8_t ifidx;
         qemu_put_8s(f, &bm->cmd);
         qemu_put_8s(f, &bm->status);
         qemu_put_be32s(f, &bm->addr);
+        qemu_put_sbe64s(f, &bm->sector_num);
+        qemu_put_be32s(f, &bm->nsector);
+        ifidx = bm->ide_if ? bm->ide_if - d->ide_if : 0;
+        qemu_put_8s(f, &ifidx);
         /* XXX: if a transfer is pending, we do not save it yet */
     }
 
@@ -3094,7 +3156,7 @@
     PCIIDEState *d = opaque;
     int ret, i;
 
-    if (version_id != 1)
+    if (version_id != 2)
         return -EINVAL;
     ret = pci_device_load(&d->dev, f);
     if (ret < 0)
@@ -3102,9 +3164,14 @@
 
     for(i = 0; i < 2; i++) {
         BMDMAState *bm = &d->bmdma[i];
+        uint8_t ifidx;
         qemu_get_8s(f, &bm->cmd);
         qemu_get_8s(f, &bm->status);
         qemu_get_be32s(f, &bm->addr);
+        qemu_get_sbe64s(f, &bm->sector_num);
+        qemu_get_be32s(f, &bm->nsector);
+        qemu_get_8s(f, &ifidx);
+        bm->ide_if = &d->ide_if[ifidx];
         /* XXX: if a transfer is pending, we do not save it yet */
     }
 
@@ -3212,7 +3279,7 @@
     ide_init2(&d->ide_if[0], hd_table[0], hd_table[1], irq[0]);
     ide_init2(&d->ide_if[2], hd_table[2], hd_table[3], irq[1]);
 
-    register_savevm("ide", 0, 1, pci_ide_save, pci_ide_load, d);
+    register_savevm("ide", 0, 2, pci_ide_save, pci_ide_load, d);
     qemu_register_reset(cmd646_reset, d);
     cmd646_reset(d);
 }
@@ -3269,7 +3336,7 @@
     ide_init_ioport(&d->ide_if[0], 0x1f0, 0x3f6);
     ide_init_ioport(&d->ide_if[2], 0x170, 0x376);
 
-    register_savevm("ide", 0, 1, pci_ide_save, pci_ide_load, d);
+    register_savevm("ide", 0, 2, pci_ide_save, pci_ide_load, d);
 }
 
 /* hd_table must contain 4 block drivers */
@@ -3308,7 +3375,7 @@
     ide_init_ioport(&d->ide_if[0], 0x1f0, 0x3f6);
     ide_init_ioport(&d->ide_if[2], 0x170, 0x376);
 
-    register_savevm("ide", 0, 1, pci_ide_save, pci_ide_load, d);
+    register_savevm("ide", 0, 2, pci_ide_save, pci_ide_load, d);
 }
 
 /***********************************************************/

Modified: trunk/sysemu.h
===================================================================
--- trunk/sysemu.h	2009-01-21 18:58:51 UTC (rev 6387)
+++ trunk/sysemu.h	2009-01-21 18:59:04 UTC (rev 6388)
@@ -128,11 +128,17 @@
     IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO
 } BlockInterfaceType;
 
+typedef enum {
+    BLOCK_ERR_REPORT, BLOCK_ERR_IGNORE, BLOCK_ERR_STOP_ENOSPC,
+    BLOCK_ERR_STOP_ANY
+} BlockInterfaceErrorAction;
+
 typedef struct DriveInfo {
     BlockDriverState *bdrv;
     BlockInterfaceType type;
     int bus;
     int unit;
+    BlockInterfaceErrorAction onerror;
     char serial[21];
 } DriveInfo;
 
@@ -146,6 +152,7 @@
 extern int drive_get_index(BlockInterfaceType type, int bus, int unit);
 extern int drive_get_max_bus(BlockInterfaceType type);
 extern const char *drive_get_serial(BlockDriverState *bdrv);
+extern BlockInterfaceErrorAction drive_get_onerror(BlockDriverState *bdrv);
 
 /* serial ports */
 

Modified: trunk/vl.c
===================================================================
--- trunk/vl.c	2009-01-21 18:58:51 UTC (rev 6387)
+++ trunk/vl.c	2009-01-21 18:59:04 UTC (rev 6388)
@@ -2200,6 +2200,17 @@
     return "\0";
 }
 
+BlockInterfaceErrorAction drive_get_onerror(BlockDriverState *bdrv)
+{
+    int index;
+
+    for (index = 0; index < nb_drives; index++)
+        if (drives_table[index].bdrv == bdrv)
+            return drives_table[index].onerror;
+
+    return BLOCK_ERR_REPORT;
+}
+
 static void bdrv_format_print(void *opaque, const char *name)
 {
     fprintf(stderr, " %s", name);
@@ -2222,12 +2233,13 @@
     int max_devs;
     int index;
     int cache;
-    int bdrv_flags;
+    int bdrv_flags, onerror;
     char *str = arg->opt;
     static const char * const params[] = { "bus", "unit", "if", "index",
                                            "cyls", "heads", "secs", "trans",
                                            "media", "snapshot", "file",
-                                           "cache", "format", "serial", NULL };
+                                           "cache", "format", "serial", "werror",
+                                           NULL };
 
     if (check_params(buf, sizeof(buf), params, str) < 0) {
          fprintf(stderr, "qemu: unknown parameter '%s' in '%s'\n",
@@ -2417,6 +2429,26 @@
     if (!get_param_value(serial, sizeof(serial), "serial", str))
 	    memset(serial, 0,  sizeof(serial));
 
+    onerror = BLOCK_ERR_REPORT;
+    if (get_param_value(buf, sizeof(serial), "werror", str)) {
+        if (type != IF_IDE) {
+            fprintf(stderr, "werror is supported only by IDE\n");
+            return -1;
+        }
+        if (!strcmp(buf, "ignore"))
+            onerror = BLOCK_ERR_IGNORE;
+        else if (!strcmp(buf, "enospc"))
+            onerror = BLOCK_ERR_STOP_ENOSPC;
+        else if (!strcmp(buf, "stop"))
+            onerror = BLOCK_ERR_STOP_ANY;
+        else if (!strcmp(buf, "report"))
+            onerror = BLOCK_ERR_REPORT;
+        else {
+            fprintf(stderr, "qemu: '%s' invalid write error action\n", buf);
+            return -1;
+        }
+    }
+
     /* compute bus and unit according index */
 
     if (index != -1) {
@@ -2480,6 +2512,7 @@
     drives_table[nb_drives].type = type;
     drives_table[nb_drives].bus = bus_id;
     drives_table[nb_drives].unit = unit_id;
+    drives_table[nb_drives].onerror = onerror;
     strncpy(drives_table[nb_drives].serial, serial, sizeof(serial));
     nb_drives++;
 

Re: [Qemu-devel] [6388] Stop VM on ENOSPC error.

[Anthony Liguori]

Anthony Liguori wrote:
> Revision: 6388
>           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6388
> Author:   aliguori
> Date:     2009-01-21 18:59:04 +0000 (Wed, 21 Jan 2009)
>
> Log Message:
> -----------
> Stop VM on ENOSPC error. (Gleb Natapov)
>
> This version of the patch adds new option "werror" to -drive flag.
> Possible values are:
>
> report    - report errors to a guest as IO errors
> ignore    - continue as if nothing happened
> stop      - stop VM on any error and retry last command on resume
> enospc    - stop vm on ENOSPC error and retry last command on resume
>             all other errors are reported to a guest.
>
> Default is "report" to maintain current behaviour.
>   

I recently got burnt by the default being "report".  I was doing an 
installation and ran out of disk space.  The guest did not do anything 
intelligible with the error reports and froze very hard (as you'd expect).

Any objection to changing to default to enospc?

Regards,

Anthony Liguori

> Signed-off-by: Gleb Natapov <gleb@redhat.com>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>   

Re: [Qemu-devel] [6388] Stop VM on ENOSPC error.

[Gleb Natapov]

On Wed, Feb 25, 2009 at 10:55:25AM -0600, Anthony Liguori wrote:
> Anthony Liguori wrote:
>> Revision: 6388
>>           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6388
>> Author:   aliguori
>> Date:     2009-01-21 18:59:04 +0000 (Wed, 21 Jan 2009)
>>
>> Log Message:
>> -----------
>> Stop VM on ENOSPC error. (Gleb Natapov)
>>
>> This version of the patch adds new option "werror" to -drive flag.
>> Possible values are:
>>
>> report    - report errors to a guest as IO errors
>> ignore    - continue as if nothing happened
>> stop      - stop VM on any error and retry last command on resume
>> enospc    - stop vm on ENOSPC error and retry last command on resume
>>             all other errors are reported to a guest.
>>
>> Default is "report" to maintain current behaviour.
>>   
>
> I recently got burnt by the default being "report".  I was doing an  
> installation and ran out of disk space.  The guest did not do anything  
> intelligible with the error reports and froze very hard (as you'd 
> expect).
>
> Any objection to changing to default to enospc?
>
Or even to stop. What guest can do with other errors anyway?

--
			Gleb.

Re: [Qemu-devel] [6388] Stop VM on ENOSPC error.

[Avi Kivity]

Anthony Liguori wrote:
>
> I recently got burnt by the default being "report".  I was doing an 
> installation and ran out of disk space.  The guest did not do anything 
> intelligible with the error reports and froze very hard (as you'd 
> expect).
>

I've been burned multiple times (before the patch went in, by the 
previous corrupt-me-disk behaviour).

> Any objection to changing to default to enospc?

I'll object to any objection!

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [6388] Stop VM on ENOSPC error.

[Daniel P. Berrange]

On Wed, Feb 25, 2009 at 10:55:25AM -0600, Anthony Liguori wrote:
> Anthony Liguori wrote:
> >Revision: 6388
> >          http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6388
> >Author:   aliguori
> >Date:     2009-01-21 18:59:04 +0000 (Wed, 21 Jan 2009)
> >
> >Log Message:
> >-----------
> >Stop VM on ENOSPC error. (Gleb Natapov)
> >
> >This version of the patch adds new option "werror" to -drive flag.
> >Possible values are:
> >
> >report    - report errors to a guest as IO errors
> >ignore    - continue as if nothing happened
> >stop      - stop VM on any error and retry last command on resume
> >enospc    - stop vm on ENOSPC error and retry last command on resume
> >            all other errors are reported to a guest.
> >
> >Default is "report" to maintain current behaviour.
> >  
> 
> I recently got burnt by the default being "report".  I was doing an 
> installation and ran out of disk space.  The guest did not do anything 
> intelligible with the error reports and froze very hard (as you'd expect).
> 
> Any objection to changing to default to enospc?

>From a managment POV having QEMU change its state from running to
paused behind our back is hard. You don't want to have to poll on
'info state' to see if the VM has paused, and QEMU provides us no
async notification for this yet. So at this time, if QEMU auto-pauses
we can't notice this change, and so again it'll just appear to the
user as if it froze.

If we get async notifications available via the monitor, then making 
the default enospc is very sensible.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [6388] Stop VM on ENOSPC error.

[Avi Kivity]

Gleb Natapov wrote:
>> Any objection to changing to default to enospc?
>>
>>     
> Or even to stop. What guest can do with other errors anyway?
>   

Softtware RAID-5 in the guest should be able to recover by reading from 
another stripe.


-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [6388] Stop VM on ENOSPC error.

[Daniel P. Berrange]

On Wed, Feb 25, 2009 at 07:04:22PM +0200, Gleb Natapov wrote:
> On Wed, Feb 25, 2009 at 10:55:25AM -0600, Anthony Liguori wrote:
> > Anthony Liguori wrote:
> >> Revision: 6388
> >>           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6388
> >> Author:   aliguori
> >> Date:     2009-01-21 18:59:04 +0000 (Wed, 21 Jan 2009)
> >>
> >> Log Message:
> >> -----------
> >> Stop VM on ENOSPC error. (Gleb Natapov)
> >>
> >> This version of the patch adds new option "werror" to -drive flag.
> >> Possible values are:
> >>
> >> report    - report errors to a guest as IO errors
> >> ignore    - continue as if nothing happened
> >> stop      - stop VM on any error and retry last command on resume
> >> enospc    - stop vm on ENOSPC error and retry last command on resume
> >>             all other errors are reported to a guest.
> >>
> >> Default is "report" to maintain current behaviour.
> >>   
> >
> > I recently got burnt by the default being "report".  I was doing an  
> > installation and ran out of disk space.  The guest did not do anything  
> > intelligible with the error reports and froze very hard (as you'd 
> > expect).
> >
> > Any objection to changing to default to enospc?
> >
> Or even to stop. What guest can do with other errors anyway?

The idea is that if the guest at least sees the I/O error, then it won't
continue writing as if everything were OK. It may not be able to continue
normal operation, but it can at least mark the FS read-only and avoid
ongoing damage. So you have a reasonable liklihood of shutting down the
guest, fixing the ENOSPC problem ont he host, and starting the guests
again & them recovering their journal.  'ignore' is guarenteed dataloss,
'report' gives you a good fighting chance. 'stop'/'enospc' are best, if
the management app is able to detect that the VM is being paused & thus
report it to the user

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [Qemu-devel] [6388] Stop VM on ENOSPC error.

[Jamie Lokier]

Avi Kivity wrote:
> Gleb Natapov wrote:
> >>Any objection to changing to default to enospc?
> >>
> >Or even to stop. What guest can do with other errors anyway?
> 
> Softtware RAID-5 in the guest should be able to recover by reading from 
> another stripe.

These are read errors, not write errors.

-- Jamie

[Qemu-devel] Re: [6388] Stop VM on ENOSPC error.

[Jan Kiszka]

Daniel P. Berrange wrote:
> On Wed, Feb 25, 2009 at 10:55:25AM -0600, Anthony Liguori wrote:
>> Anthony Liguori wrote:
>>> Revision: 6388
>>>          http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6388
>>> Author:   aliguori
>>> Date:     2009-01-21 18:59:04 +0000 (Wed, 21 Jan 2009)
>>>
>>> Log Message:
>>> -----------
>>> Stop VM on ENOSPC error. (Gleb Natapov)
>>>
>>> This version of the patch adds new option "werror" to -drive flag.
>>> Possible values are:
>>>
>>> report    - report errors to a guest as IO errors
>>> ignore    - continue as if nothing happened
>>> stop      - stop VM on any error and retry last command on resume
>>> enospc    - stop vm on ENOSPC error and retry last command on resume
>>>            all other errors are reported to a guest.
>>>
>>> Default is "report" to maintain current behaviour.
>>>  
>> I recently got burnt by the default being "report".  I was doing an 
>> installation and ran out of disk space.  The guest did not do anything 
>> intelligible with the error reports and froze very hard (as you'd expect).
>>
>> Any objection to changing to default to enospc?
> 
> From a managment POV having QEMU change its state from running to
> paused behind our back is hard. You don't want to have to poll on
> 'info state' to see if the VM has paused, and QEMU provides us no
> async notification for this yet. So at this time, if QEMU auto-pauses
> we can't notice this change, and so again it'll just appear to the
> user as if it froze.
> 
> If we get async notifications available via the monitor, then making 
> the default enospc is very sensible.

Adding some "notify <event-list>" monitor command could be a way to
allow this without bothering every monitor user immediately (I'm
thinking of multiple, decoupled monitor terminals now). Well, this
particular error might be critical enough to let it pop up everywhere,
but I guess there could be less critical ones reported this way, too.

When implementing async notification, we just have to ensure that we
never push it between incomplete monitor output.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [6388] Stop VM on ENOSPC error.

[Anthony Liguori]

Avi Kivity wrote:
> Anthony Liguori wrote:
>>
>> I recently got burnt by the default being "report".  I was doing an 
>> installation and ran out of disk space.  The guest did not do 
>> anything intelligible with the error reports and froze very hard (as 
>> you'd expect).
>>
>
> I've been burned multiple times (before the patch went in, by the 
> previous corrupt-me-disk behaviour).

:-)  I'm curious, has "report" ever saved you?  My guest went completely 
fubar.  Have you ever run into a disk error that wasn't ENOSPC?

Regards,

Anthony Liguori

>> Any objection to changing to default to enospc?
>
> I'll object to any objection!
>

Re: [Qemu-devel] [6388] Stop VM on ENOSPC error.

[Anthony Liguori]

Daniel P. Berrange wrote:
> On Wed, Feb 25, 2009 at 10:55:25AM -0600, Anthony Liguori wrote:
>   
>> Anthony Liguori wrote:
>>     
>>> Revision: 6388
>>>          http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=6388
>>> Author:   aliguori
>>> Date:     2009-01-21 18:59:04 +0000 (Wed, 21 Jan 2009)
>>>
>>> Log Message:
>>> -----------
>>> Stop VM on ENOSPC error. (Gleb Natapov)
>>>
>>> This version of the patch adds new option "werror" to -drive flag.
>>> Possible values are:
>>>
>>> report    - report errors to a guest as IO errors
>>> ignore    - continue as if nothing happened
>>> stop      - stop VM on any error and retry last command on resume
>>> enospc    - stop vm on ENOSPC error and retry last command on resume
>>>            all other errors are reported to a guest.
>>>
>>> Default is "report" to maintain current behaviour.
>>>  
>>>       
>> I recently got burnt by the default being "report".  I was doing an 
>> installation and ran out of disk space.  The guest did not do anything 
>> intelligible with the error reports and froze very hard (as you'd expect).
>>
>> Any objection to changing to default to enospc?
>>     
>
> From a managment POV having QEMU change its state from running to
> paused behind our back is hard. You don't want to have to poll on
> 'info state' to see if the VM has paused, and QEMU provides us no
> async notification for this yet. So at this time, if QEMU auto-pauses
> we can't notice this change, and so again it'll just appear to the
> user as if it froze.
>   

I appreciate the difficulty but I don't think report is very much better 
from your perspective if it results in the guest completely freezing.

Regards,

Anthony Liguori

> If we get async notifications available via the monitor, then making 
> the default enospc is very sensible.
>
> Daniel
>   

Re: [Qemu-devel] [6388] Stop VM on ENOSPC error.

[Anthony Liguori]

Daniel P. Berrange wrote:
> On Wed, Feb 25, 2009 at 07:04:22PM +0200, Gleb Natapov wrote:
>   
>> Or even to stop. What guest can do with other errors anyway?
>>     
>
> The idea is that if the guest at least sees the I/O error, then it won't
> continue writing as if everything were OK. It may not be able to continue
> normal operation, but it can at least mark the FS read-only and avoid
> ongoing damage.

This simply doesn't happen in practice.  The FS needs to write data to 
the disk in order to remount it read-only.  Once all writes start 
failing, the errors are cascading.

>  So you have a reasonable liklihood of shutting down the
> guest, fixing the ENOSPC problem ont he host, and starting the guests
> again & them recovering their journal.  'ignore' is guarenteed dataloss,
> 'report' gives you a good fighting chance. 'stop'/'enospc' are best, if
> the management app is able to detect that the VM is being paused & thus
> report it to the user
>   
My contention is that a user using a management app is already hosed 
with "report".  To not be fubar, we need async notifications.  If we can 
the default to enospc, I think it makes nothing worse but a good chunk 
of cases a lot better.

Regards,

Anthony Liguori


> Daniel
>   

Re: [Qemu-devel] [6388] Stop VM on ENOSPC error.

[Jamie Lokier]

Daniel P. Berrange wrote:
> > Or even to stop. What guest can do with other errors anyway?
> 
> The idea is that if the guest at least sees the I/O error, then it won't
> continue writing as if everything were OK. It may not be able to continue
> normal operation, but it can at least mark the FS read-only and avoid
> ongoing damage. So you have a reasonable liklihood of shutting down the
> guest, fixing the ENOSPC problem ont he host, and starting the guests
> again & them recovering their journal.  'ignore' is guarenteed dataloss,
> 'report' gives you a good fighting chance. 'stop'/'enospc' are best, if
> the management app is able to detect that the VM is being paused & thus
> report it to the user

Even Linux guests don't respond to an I/O error quite as above.  In
Linux, the I/O queue (elevator) may have additional pending write
commands which are executed despite an earlier "journal" write
failing.  This can result in an inconsistent filesystem if one write
fails and later ones succeed.  It's equivalent to losing power and
barriers not being honoured.

If QEMU were to have a "sticky" error flag which turns all writes into
I/O errors after a failed one, even if the host ENOSPC is transient,
that would be better for guest data consistency.  (Not perfect for
transactions spanning multiple disks (etc.), but good for a single
journalled filesystem).

-- Jamie

Re: [Qemu-devel] [6388] Stop VM on ENOSPC error.

[Jamie Lokier]

Anthony Liguori wrote:
> >The idea is that if the guest at least sees the I/O error, then it won't
> >continue writing as if everything were OK. It may not be able to continue
> >normal operation, but it can at least mark the FS read-only and avoid
> >ongoing damage.
> 
> This simply doesn't happen in practice.  The FS needs to write data to 
> the disk in order to remount it read-only.  Once all writes start 
> failing, the errors are cascading.

ext2/3/4 have documented errors=remount-ro / errors=panic mount options.

Don't they work?

(I know that journal integrity is not _exactly_ maintained when write
errors occur at awkward times, as described in my previous mail, but
apart from that don't the above options work?)

-- Jamie

Re: [Qemu-devel] [6388] Stop VM on ENOSPC error.

[Jamie Lokier]

Daniel P. Berrange wrote:
> > Any objection to changing to default to enospc?
> 
> >From a managment POV having QEMU change its state from running to
> paused behind our back is hard.

>From a management POV, the manager can just set the behaviour to
'report' if that's what you prefer :-)

> If we get async notifications available via the monitor, then making 
> the default enospc is very sensible.

Async notifications are obviously best, but can QEMU's logging be
used?  Can it log the ENOSPC event, and the management app watch the log?

-- Jamie

Re: [Qemu-devel] [6388] Stop VM on ENOSPC error.

[Anthony Liguori]

Jamie Lokier wrote:
> Anthony Liguori wrote:
>   
>>> The idea is that if the guest at least sees the I/O error, then it won't
>>> continue writing as if everything were OK. It may not be able to continue
>>> normal operation, but it can at least mark the FS read-only and avoid
>>> ongoing damage.
>>>       
>> This simply doesn't happen in practice.  The FS needs to write data to 
>> the disk in order to remount it read-only.  Once all writes start 
>> failing, the errors are cascading.
>>     
>
> ext2/3/4 have documented errors=remount-ro / errors=panic mount options.
>
> Don't they work?
>   

This was a sles10sp1 guest which uses reiserfs by default.

Regards,

Anthony Liguori

Re: [Qemu-devel] [6388] Stop VM on ENOSPC error.

[Avi Kivity]

Anthony Liguori wrote:
> Avi Kivity wrote:
>> Anthony Liguori wrote:
>>>
>>> I recently got burnt by the default being "report".  I was doing an 
>>> installation and ran out of disk space.  The guest did not do 
>>> anything intelligible with the error reports and froze very hard (as 
>>> you'd expect).
>>>
>>
>> I've been burned multiple times (before the patch went in, by the 
>> previous corrupt-me-disk behaviour).
>
> :-)  I'm curious, has "report" ever saved you?  My guest went 
> completely fubar.  Have you ever run into a disk error that wasn't 
> ENOSPC?
>

Either this happened before the patch went in, or I didn't look at the 
monitor, so it hasn't.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [6388] Stop VM on ENOSPC error.

[Avi Kivity]

Jamie Lokier wrote:
> Avi Kivity wrote:
>   
>> Gleb Natapov wrote:
>>     
>>>> Any objection to changing to default to enospc?
>>>>
>>>>         
>>> Or even to stop. What guest can do with other errors anyway?
>>>       
>> Softtware RAID-5 in the guest should be able to recover by reading from 
>> another stripe.
>>     
>
> These are read errors, not write errors.
>   

I thought 'stop' considered all errors.


-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [6388] Stop VM on ENOSPC error.

[Gleb Natapov]

On Thu, Feb 26, 2009 at 11:20:52AM +0200, Avi Kivity wrote:
> Jamie Lokier wrote:
>> Avi Kivity wrote:
>>   
>>> Gleb Natapov wrote:
>>>     
>>>>> Any objection to changing to default to enospc?
>>>>>
>>>>>         
>>>> Or even to stop. What guest can do with other errors anyway?
>>>>       
>>> Softtware RAID-5 in the guest should be able to recover by reading 
>>> from another stripe.
>>>     
>>
>> These are read errors, not write errors.
>>   
>
> I thought 'stop' considered all errors.
>
>
No, only write errors. That is why parameter is called werror. I thought
that handle write errors and read errors differently may be useful.

--
			Gleb.