| Message ID | 1274889109-9925-1-git-send-email-agraf@suse.de |
|---|---|
| State | New |
| Headers | show |
On 05/26/2010 10:51 AM, Alexander Graf wrote: > Usually the guest can tell the host to flush data to disk. In some cases we > don't want to flush though, but try to keep everything in cache. > > So let's add a new cache value to -drive that allows us to set the cache > policy to most aggressive, disabling flushes. We call this mode "unsafe", > as guest data is not guaranteed to survive host crashes anymore. > > This patch also adds a noop function for aio, so we can do nothing in AIO > fashion. > > Signed-off-by: Alexander Graf<agraf@suse.de> > I'm happy merging this for now, but I think it might make more sense to do this as per-device flush control qdev options since those bits are actually guest visible. Christoph, does that seem reasonable? Regards, Anthony Liguori > --- > > v2 -> v3: > > - Add description of cache=volatile > - Squash aio noop noop patch into this one > > v3 -> v4: > > - Rename cache=volatile to cache=unsafe > --- > block.c | 28 ++++++++++++++++++++++++++++ > block.h | 1 + > qemu-config.c | 2 +- > qemu-options.hx | 13 ++++++++++--- > vl.c | 3 +++ > 5 files changed, 43 insertions(+), 4 deletions(-) > > diff --git a/block.c b/block.c > index 0b0966c..3c0249b 100644 > --- a/block.c > +++ b/block.c > @@ -50,6 +50,8 @@ static BlockDriverAIOCB *bdrv_aio_writev_em(BlockDriverState *bs, > BlockDriverCompletionFunc *cb, void *opaque); > static BlockDriverAIOCB *bdrv_aio_flush_em(BlockDriverState *bs, > BlockDriverCompletionFunc *cb, void *opaque); > +static BlockDriverAIOCB *bdrv_aio_noop_em(BlockDriverState *bs, > + BlockDriverCompletionFunc *cb, void *opaque); > static int bdrv_read_em(BlockDriverState *bs, int64_t sector_num, > uint8_t *buf, int nb_sectors); > static int bdrv_write_em(BlockDriverState *bs, int64_t sector_num, > @@ -1312,6 +1314,10 @@ const char *bdrv_get_device_name(BlockDriverState *bs) > > void bdrv_flush(BlockDriverState *bs) > { > + if (bs->open_flags& BDRV_O_NO_FLUSH) { > + return; > + } > + > if (bs->drv&& bs->drv->bdrv_flush) > bs->drv->bdrv_flush(bs); > } > @@ -2099,6 +2105,10 @@ BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs, > { > BlockDriver *drv = bs->drv; > > + if (bs->open_flags& BDRV_O_NO_FLUSH) { > + return bdrv_aio_noop_em(bs, cb, opaque); > + } > + > if (!drv) > return NULL; > return drv->bdrv_aio_flush(bs, cb, opaque); > @@ -2214,6 +2224,24 @@ static BlockDriverAIOCB *bdrv_aio_flush_em(BlockDriverState *bs, > return&acb->common; > } > > +static BlockDriverAIOCB *bdrv_aio_noop_em(BlockDriverState *bs, > + BlockDriverCompletionFunc *cb, void *opaque) > +{ > + BlockDriverAIOCBSync *acb; > + > + acb = qemu_aio_get(&bdrv_em_aio_pool, bs, cb, opaque); > + acb->is_write = 1; /* don't bounce in the completion hadler */ > + acb->qiov = NULL; > + acb->bounce = NULL; > + acb->ret = 0; > + > + if (!acb->bh) > + acb->bh = qemu_bh_new(bdrv_aio_bh_cb, acb); > + > + qemu_bh_schedule(acb->bh); > + return&acb->common; > +} > + > /**************************************************************/ > /* sync block device emulation */ > > diff --git a/block.h b/block.h > index 278259c..24efeb6 100644 > --- a/block.h > +++ b/block.h > @@ -33,6 +33,7 @@ typedef struct QEMUSnapshotInfo { > #define BDRV_O_CACHE_WB 0x0040 /* use write-back caching */ > #define BDRV_O_NATIVE_AIO 0x0080 /* use native AIO instead of the thread pool */ > #define BDRV_O_NO_BACKING 0x0100 /* don't open the backing file */ > +#define BDRV_O_NO_FLUSH 0x0200 /* disable flushing on this disk */ > > #define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_CACHE_WB) > > diff --git a/qemu-config.c b/qemu-config.c > index aa376d4..5a4e61b 100644 > --- a/qemu-config.c > +++ b/qemu-config.c > @@ -54,7 +54,7 @@ QemuOptsList qemu_drive_opts = { > },{ > .name = "cache", > .type = QEMU_OPT_STRING, > - .help = "host cache usage (none, writeback, writethrough)", > + .help = "host cache usage (none, writeback, writethrough, unsafe)", > },{ > .name = "aio", > .type = QEMU_OPT_STRING, > diff --git a/qemu-options.hx b/qemu-options.hx > index 03e95fd..cea9b72 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -118,8 +118,9 @@ ETEXI > DEF("drive", HAS_ARG, QEMU_OPTION_drive, > "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n" > " [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n" > - " [,cache=writethrough|writeback|none][,format=f][,serial=s]\n" > - " [,addr=A][,id=name][,aio=threads|native][,readonly=on|off]\n" > + " [,cache=writethrough|writeback|unsafe|none][,format=f]\n" > + " [,serial=s][,addr=A][,id=name][,aio=threads|native]\n" > + " [,readonly=on|off]\n" > " use 'file' as a drive image\n", QEMU_ARCH_ALL) > STEXI > @item -drive @var{option}[,@var{option}[,@var{option}[,...]]] > @@ -148,7 +149,7 @@ These options have the same definition as they have in @option{-hdachs}. > @item snapshot=@var{snapshot} > @var{snapshot} is "on" or "off" and allows to enable snapshot for given drive (see @option{-snapshot}). > @item cache=@var{cache} > -@var{cache} is "none", "writeback", or "writethrough" and controls how the host cache is used to access block data. > +@var{cache} is "none", "writeback", "unsafe", or "writethrough" and controls how the host cache is used to access block data. > @item aio=@var{aio} > @var{aio} is "threads", or "native" and selects between pthread based disk I/O and native Linux AIO. > @item format=@var{format} > @@ -180,6 +181,12 @@ Some block drivers perform badly with @option{cache=writethrough}, most notably, > qcow2. If performance is more important than correctness, > @option{cache=writeback} should be used with qcow2. > > +In case you don't care about data integrity over host failures, use > +cache=unsafe. This option tells qemu that it never needs to write any data > +to the disk but can instead keeps things in cache. If anything goes wrong, > +like your host losing power, the disk storage getting disconnected accidently, > +etc. you're image will most probably be rendered unusable. > + > Instead of @option{-cdrom} you can use: > @example > qemu -drive file=file,index=2,media=cdrom > diff --git a/vl.c b/vl.c > index 328395e..bc15dd7 100644 > --- a/vl.c > +++ b/vl.c > @@ -913,6 +913,9 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque, > bdrv_flags |= BDRV_O_NOCACHE; > } else if (!strcmp(buf, "writeback")) { > bdrv_flags |= BDRV_O_CACHE_WB; > + } else if (!strcmp(buf, "unsafe")) { > + bdrv_flags |= BDRV_O_CACHE_WB; > + bdrv_flags |= BDRV_O_NO_FLUSH; > } else if (!strcmp(buf, "writethrough")) { > /* this is the default */ > } else { >
On 26.05.2010, at 18:17, Anthony Liguori wrote: > On 05/26/2010 10:51 AM, Alexander Graf wrote: >> Usually the guest can tell the host to flush data to disk. In some cases we >> don't want to flush though, but try to keep everything in cache. >> >> So let's add a new cache value to -drive that allows us to set the cache >> policy to most aggressive, disabling flushes. We call this mode "unsafe", >> as guest data is not guaranteed to survive host crashes anymore. >> >> This patch also adds a noop function for aio, so we can do nothing in AIO >> fashion. >> >> Signed-off-by: Alexander Graf<agraf@suse.de> >> > > I'm happy merging this for now, but I think it might make more sense to do this as per-device flush control qdev options since those bits are actually guest visible. Christoph, does that seem reasonable? The way it's implemented in this patch it's not guest visible. The guest still gets the flush capability exposed, but the actual operation returns a nop. With cache=unsafe qemu basically bevaves the same as a battery backed raid controller. Alex
On Wed, May 26, 2010 at 05:51:49PM +0200, Alexander Graf wrote: > Usually the guest can tell the host to flush data to disk. In some cases we > don't want to flush though, but try to keep everything in cache. > > So let's add a new cache value to -drive that allows us to set the cache > policy to most aggressive, disabling flushes. We call this mode "unsafe", > as guest data is not guaranteed to survive host crashes anymore. > > This patch also adds a noop function for aio, so we can do nothing in AIO > fashion. > > Signed-off-by: Alexander Graf <agraf@suse.de> It seems we finally reached a consensus here, so I have applied the patch. > --- > > v2 -> v3: > > - Add description of cache=volatile > - Squash aio noop noop patch into this one > > v3 -> v4: > > - Rename cache=volatile to cache=unsafe > --- > block.c | 28 ++++++++++++++++++++++++++++ > block.h | 1 + > qemu-config.c | 2 +- > qemu-options.hx | 13 ++++++++++--- > vl.c | 3 +++ > 5 files changed, 43 insertions(+), 4 deletions(-) > > diff --git a/block.c b/block.c > index 0b0966c..3c0249b 100644 > --- a/block.c > +++ b/block.c > @@ -50,6 +50,8 @@ static BlockDriverAIOCB *bdrv_aio_writev_em(BlockDriverState *bs, > BlockDriverCompletionFunc *cb, void *opaque); > static BlockDriverAIOCB *bdrv_aio_flush_em(BlockDriverState *bs, > BlockDriverCompletionFunc *cb, void *opaque); > +static BlockDriverAIOCB *bdrv_aio_noop_em(BlockDriverState *bs, > + BlockDriverCompletionFunc *cb, void *opaque); > static int bdrv_read_em(BlockDriverState *bs, int64_t sector_num, > uint8_t *buf, int nb_sectors); > static int bdrv_write_em(BlockDriverState *bs, int64_t sector_num, > @@ -1312,6 +1314,10 @@ const char *bdrv_get_device_name(BlockDriverState *bs) > > void bdrv_flush(BlockDriverState *bs) > { > + if (bs->open_flags & BDRV_O_NO_FLUSH) { > + return; > + } > + > if (bs->drv && bs->drv->bdrv_flush) > bs->drv->bdrv_flush(bs); > } > @@ -2099,6 +2105,10 @@ BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs, > { > BlockDriver *drv = bs->drv; > > + if (bs->open_flags & BDRV_O_NO_FLUSH) { > + return bdrv_aio_noop_em(bs, cb, opaque); > + } > + > if (!drv) > return NULL; > return drv->bdrv_aio_flush(bs, cb, opaque); > @@ -2214,6 +2224,24 @@ static BlockDriverAIOCB *bdrv_aio_flush_em(BlockDriverState *bs, > return &acb->common; > } > > +static BlockDriverAIOCB *bdrv_aio_noop_em(BlockDriverState *bs, > + BlockDriverCompletionFunc *cb, void *opaque) > +{ > + BlockDriverAIOCBSync *acb; > + > + acb = qemu_aio_get(&bdrv_em_aio_pool, bs, cb, opaque); > + acb->is_write = 1; /* don't bounce in the completion hadler */ > + acb->qiov = NULL; > + acb->bounce = NULL; > + acb->ret = 0; > + > + if (!acb->bh) > + acb->bh = qemu_bh_new(bdrv_aio_bh_cb, acb); > + > + qemu_bh_schedule(acb->bh); > + return &acb->common; > +} > + > /**************************************************************/ > /* sync block device emulation */ > > diff --git a/block.h b/block.h > index 278259c..24efeb6 100644 > --- a/block.h > +++ b/block.h > @@ -33,6 +33,7 @@ typedef struct QEMUSnapshotInfo { > #define BDRV_O_CACHE_WB 0x0040 /* use write-back caching */ > #define BDRV_O_NATIVE_AIO 0x0080 /* use native AIO instead of the thread pool */ > #define BDRV_O_NO_BACKING 0x0100 /* don't open the backing file */ > +#define BDRV_O_NO_FLUSH 0x0200 /* disable flushing on this disk */ > > #define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_CACHE_WB) > > diff --git a/qemu-config.c b/qemu-config.c > index aa376d4..5a4e61b 100644 > --- a/qemu-config.c > +++ b/qemu-config.c > @@ -54,7 +54,7 @@ QemuOptsList qemu_drive_opts = { > },{ > .name = "cache", > .type = QEMU_OPT_STRING, > - .help = "host cache usage (none, writeback, writethrough)", > + .help = "host cache usage (none, writeback, writethrough, unsafe)", > },{ > .name = "aio", > .type = QEMU_OPT_STRING, > diff --git a/qemu-options.hx b/qemu-options.hx > index 03e95fd..cea9b72 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -118,8 +118,9 @@ ETEXI > DEF("drive", HAS_ARG, QEMU_OPTION_drive, > "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n" > " [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n" > - " [,cache=writethrough|writeback|none][,format=f][,serial=s]\n" > - " [,addr=A][,id=name][,aio=threads|native][,readonly=on|off]\n" > + " [,cache=writethrough|writeback|unsafe|none][,format=f]\n" > + " [,serial=s][,addr=A][,id=name][,aio=threads|native]\n" > + " [,readonly=on|off]\n" > " use 'file' as a drive image\n", QEMU_ARCH_ALL) > STEXI > @item -drive @var{option}[,@var{option}[,@var{option}[,...]]] > @@ -148,7 +149,7 @@ These options have the same definition as they have in @option{-hdachs}. > @item snapshot=@var{snapshot} > @var{snapshot} is "on" or "off" and allows to enable snapshot for given drive (see @option{-snapshot}). > @item cache=@var{cache} > -@var{cache} is "none", "writeback", or "writethrough" and controls how the host cache is used to access block data. > +@var{cache} is "none", "writeback", "unsafe", or "writethrough" and controls how the host cache is used to access block data. > @item aio=@var{aio} > @var{aio} is "threads", or "native" and selects between pthread based disk I/O and native Linux AIO. > @item format=@var{format} > @@ -180,6 +181,12 @@ Some block drivers perform badly with @option{cache=writethrough}, most notably, > qcow2. If performance is more important than correctness, > @option{cache=writeback} should be used with qcow2. > > +In case you don't care about data integrity over host failures, use > +cache=unsafe. This option tells qemu that it never needs to write any data > +to the disk but can instead keeps things in cache. If anything goes wrong, > +like your host losing power, the disk storage getting disconnected accidently, > +etc. you're image will most probably be rendered unusable. > + > Instead of @option{-cdrom} you can use: > @example > qemu -drive file=file,index=2,media=cdrom > diff --git a/vl.c b/vl.c > index 328395e..bc15dd7 100644 > --- a/vl.c > +++ b/vl.c > @@ -913,6 +913,9 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque, > bdrv_flags |= BDRV_O_NOCACHE; > } else if (!strcmp(buf, "writeback")) { > bdrv_flags |= BDRV_O_CACHE_WB; > + } else if (!strcmp(buf, "unsafe")) { > + bdrv_flags |= BDRV_O_CACHE_WB; > + bdrv_flags |= BDRV_O_NO_FLUSH; > } else if (!strcmp(buf, "writethrough")) { > /* this is the default */ > } else { > -- > 1.6.0.2 > > >
Anthony Liguori <anthony@codemonkey.ws> writes: > On 05/26/2010 10:51 AM, Alexander Graf wrote: >> Usually the guest can tell the host to flush data to disk. In some cases we >> don't want to flush though, but try to keep everything in cache. >> >> So let's add a new cache value to -drive that allows us to set the cache >> policy to most aggressive, disabling flushes. We call this mode "unsafe", >> as guest data is not guaranteed to survive host crashes anymore. >> >> This patch also adds a noop function for aio, so we can do nothing in AIO >> fashion. >> >> Signed-off-by: Alexander Graf<agraf@suse.de> >> > > I'm happy merging this for now, but I think it might make more sense > to do this as per-device flush control qdev options since those bits > are actually guest visible. Christoph, does that seem reasonable? The qdev property's default could be based on the host block device cache option. This is similar to how I intend to do readonly: if qdev's readonly property is unset, match host block device's readonly option. If qdev's readonly property is off, host block device's must be off, too. If it's on, anything goes. Some qdevs don't have a readonly property, because their readonlyness is fixed, but that's detail.
diff --git a/block.c b/block.c index 0b0966c..3c0249b 100644 --- a/block.c +++ b/block.c @@ -50,6 +50,8 @@ static BlockDriverAIOCB *bdrv_aio_writev_em(BlockDriverState *bs, BlockDriverCompletionFunc *cb, void *opaque); static BlockDriverAIOCB *bdrv_aio_flush_em(BlockDriverState *bs, BlockDriverCompletionFunc *cb, void *opaque); +static BlockDriverAIOCB *bdrv_aio_noop_em(BlockDriverState *bs, + BlockDriverCompletionFunc *cb, void *opaque); static int bdrv_read_em(BlockDriverState *bs, int64_t sector_num, uint8_t *buf, int nb_sectors); static int bdrv_write_em(BlockDriverState *bs, int64_t sector_num, @@ -1312,6 +1314,10 @@ const char *bdrv_get_device_name(BlockDriverState *bs) void bdrv_flush(BlockDriverState *bs) { + if (bs->open_flags & BDRV_O_NO_FLUSH) { + return; + } + if (bs->drv && bs->drv->bdrv_flush) bs->drv->bdrv_flush(bs); } @@ -2099,6 +2105,10 @@ BlockDriverAIOCB *bdrv_aio_flush(BlockDriverState *bs, { BlockDriver *drv = bs->drv; + if (bs->open_flags & BDRV_O_NO_FLUSH) { + return bdrv_aio_noop_em(bs, cb, opaque); + } + if (!drv) return NULL; return drv->bdrv_aio_flush(bs, cb, opaque); @@ -2214,6 +2224,24 @@ static BlockDriverAIOCB *bdrv_aio_flush_em(BlockDriverState *bs, return &acb->common; } +static BlockDriverAIOCB *bdrv_aio_noop_em(BlockDriverState *bs, + BlockDriverCompletionFunc *cb, void *opaque) +{ + BlockDriverAIOCBSync *acb; + + acb = qemu_aio_get(&bdrv_em_aio_pool, bs, cb, opaque); + acb->is_write = 1; /* don't bounce in the completion hadler */ + acb->qiov = NULL; + acb->bounce = NULL; + acb->ret = 0; + + if (!acb->bh) + acb->bh = qemu_bh_new(bdrv_aio_bh_cb, acb); + + qemu_bh_schedule(acb->bh); + return &acb->common; +} + /**************************************************************/ /* sync block device emulation */ diff --git a/block.h b/block.h index 278259c..24efeb6 100644 --- a/block.h +++ b/block.h @@ -33,6 +33,7 @@ typedef struct QEMUSnapshotInfo { #define BDRV_O_CACHE_WB 0x0040 /* use write-back caching */ #define BDRV_O_NATIVE_AIO 0x0080 /* use native AIO instead of the thread pool */ #define BDRV_O_NO_BACKING 0x0100 /* don't open the backing file */ +#define BDRV_O_NO_FLUSH 0x0200 /* disable flushing on this disk */ #define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_CACHE_WB) diff --git a/qemu-config.c b/qemu-config.c index aa376d4..5a4e61b 100644 --- a/qemu-config.c +++ b/qemu-config.c @@ -54,7 +54,7 @@ QemuOptsList qemu_drive_opts = { },{ .name = "cache", .type = QEMU_OPT_STRING, - .help = "host cache usage (none, writeback, writethrough)", + .help = "host cache usage (none, writeback, writethrough, unsafe)", },{ .name = "aio", .type = QEMU_OPT_STRING, diff --git a/qemu-options.hx b/qemu-options.hx index 03e95fd..cea9b72 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -118,8 +118,9 @@ ETEXI DEF("drive", HAS_ARG, QEMU_OPTION_drive, "-drive [file=file][,if=type][,bus=n][,unit=m][,media=d][,index=i]\n" " [,cyls=c,heads=h,secs=s[,trans=t]][,snapshot=on|off]\n" - " [,cache=writethrough|writeback|none][,format=f][,serial=s]\n" - " [,addr=A][,id=name][,aio=threads|native][,readonly=on|off]\n" + " [,cache=writethrough|writeback|unsafe|none][,format=f]\n" + " [,serial=s][,addr=A][,id=name][,aio=threads|native]\n" + " [,readonly=on|off]\n" " use 'file' as a drive image\n", QEMU_ARCH_ALL) STEXI @item -drive @var{option}[,@var{option}[,@var{option}[,...]]] @@ -148,7 +149,7 @@ These options have the same definition as they have in @option{-hdachs}. @item snapshot=@var{snapshot} @var{snapshot} is "on" or "off" and allows to enable snapshot for given drive (see @option{-snapshot}). @item cache=@var{cache} -@var{cache} is "none", "writeback", or "writethrough" and controls how the host cache is used to access block data. +@var{cache} is "none", "writeback", "unsafe", or "writethrough" and controls how the host cache is used to access block data. @item aio=@var{aio} @var{aio} is "threads", or "native" and selects between pthread based disk I/O and native Linux AIO. @item format=@var{format} @@ -180,6 +181,12 @@ Some block drivers perform badly with @option{cache=writethrough}, most notably, qcow2. If performance is more important than correctness, @option{cache=writeback} should be used with qcow2. +In case you don't care about data integrity over host failures, use +cache=unsafe. This option tells qemu that it never needs to write any data +to the disk but can instead keeps things in cache. If anything goes wrong, +like your host losing power, the disk storage getting disconnected accidently, +etc. you're image will most probably be rendered unusable. + Instead of @option{-cdrom} you can use: @example qemu -drive file=file,index=2,media=cdrom diff --git a/vl.c b/vl.c index 328395e..bc15dd7 100644 --- a/vl.c +++ b/vl.c @@ -913,6 +913,9 @@ DriveInfo *drive_init(QemuOpts *opts, void *opaque, bdrv_flags |= BDRV_O_NOCACHE; } else if (!strcmp(buf, "writeback")) { bdrv_flags |= BDRV_O_CACHE_WB; + } else if (!strcmp(buf, "unsafe")) { + bdrv_flags |= BDRV_O_CACHE_WB; + bdrv_flags |= BDRV_O_NO_FLUSH; } else if (!strcmp(buf, "writethrough")) { /* this is the default */ } else {
Usually the guest can tell the host to flush data to disk. In some cases we don't want to flush though, but try to keep everything in cache. So let's add a new cache value to -drive that allows us to set the cache policy to most aggressive, disabling flushes. We call this mode "unsafe", as guest data is not guaranteed to survive host crashes anymore. This patch also adds a noop function for aio, so we can do nothing in AIO fashion. Signed-off-by: Alexander Graf <agraf@suse.de> --- v2 -> v3: - Add description of cache=volatile - Squash aio noop noop patch into this one v3 -> v4: - Rename cache=volatile to cache=unsafe --- block.c | 28 ++++++++++++++++++++++++++++ block.h | 1 + qemu-config.c | 2 +- qemu-options.hx | 13 ++++++++++--- vl.c | 3 +++ 5 files changed, 43 insertions(+), 4 deletions(-)