| Message ID | d2e6189248c6e4c2d04f849d57409952ff79da94.1429272036.git.bv.trach@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 04/17/2015 06:13 AM, Bohdan Trach wrote: > From: Bohdan Trach <bohdan.trach@mailbox.tu-dresden.de> > > dump-pc-mem command is added for checkpointing guest memory to > file. Only system RAM region is saved. This checkpoint is later used to > recover unchanged pages. > > Signed-off-by: Bohdan Trach <bohdan.trach@mailbox.tu-dresden.de> > --- > > +void qmp_dump_pc_ram(const char *file, Error **errp) { > + > + int rc; > + int fd; > + fd = open(file, Please use qemu_open() rather than raw open(), so that your command automatically supports /dev/fdset/nnn notation for reusing a file descriptor passed in via SCM_RIGHTS. > +++ b/qapi-schema.json > @@ -3648,3 +3648,14 @@ > # Since: 2.1 > ## > { 'command': 'rtc-reset-reinjection' } > + > +## > +# @dump-pc-ram: > +# > +# Checkpoints guest. The whole guest, or just guest memory? > +# > +# @file: the file to save the memory to as binary data > +# > +# Returns: Nothing on success Missing a 'Since: 2.4' designation. > +## > +{ 'command': 'dump-pc-ram', 'data': {'file': 'str'} }
Thank You for the review. Please see comments inline. On 04/17/2015 03:53 PM, Eric Blake wrote: > On 04/17/2015 06:13 AM, Bohdan Trach wrote: >> From: Bohdan Trach <bohdan.trach@mailbox.tu-dresden.de> >> >> dump-pc-mem command is added for checkpointing guest memory to >> file. Only system RAM region is saved. This checkpoint is later used to >> recover unchanged pages. >> >> Signed-off-by: Bohdan Trach <bohdan.trach@mailbox.tu-dresden.de> >> --- > >> >> +void qmp_dump_pc_ram(const char *file, Error **errp) { >> + >> + int rc; >> + int fd; >> + fd = open(file, > > Please use qemu_open() rather than raw open(), so that your command > automatically supports /dev/fdset/nnn notation for reusing a file > descriptor passed in via SCM_RIGHTS. > OK, this will be fixed. >> +++ b/qapi-schema.json >> @@ -3648,3 +3648,14 @@ >> # Since: 2.1 >> ## >> { 'command': 'rtc-reset-reinjection' } >> + >> +## >> +# @dump-pc-ram: >> +# >> +# Checkpoints guest. > > The whole guest, or just guest memory? > dump-pc-ram command currently writes "pc.ram" MemoryRegion to the specified file. >> +# >> +# @file: the file to save the memory to as binary data >> +# >> +# Returns: Nothing on success > > Missing a 'Since: 2.4' designation. > OK, I'll add it.
* Bohdan Trach (bv.trach@gmail.com) wrote: > From: Bohdan Trach <bohdan.trach@mailbox.tu-dresden.de> > > dump-pc-mem command is added for checkpointing guest memory to > file. Only system RAM region is saved. This checkpoint is later used to > recover unchanged pages. Why not just use the 'dump_guest_memory' commands; they dump it in interesting existing formats; they have headers in the files as well rather than just a raw blob of data. If you wanted to restrict to only certain RAM blocks, then I'd suggest adding a feature to that existing command. You might also find that you want other RAMBlocks as well, for example where RAM is added using hot plu, those are separate RAM blocks. Dave > Signed-off-by: Bohdan Trach <bohdan.trach@mailbox.tu-dresden.de> > --- > arch_init.c | 19 +++++++++++++++++++ > hmp-commands.hx | 16 ++++++++++++++++ > hmp.c | 9 +++++++++ > hmp.h | 1 + > qapi-schema.json | 11 +++++++++++ > qmp-commands.hx | 19 +++++++++++++++++++ > 6 files changed, 75 insertions(+) > > diff --git a/arch_init.c b/arch_init.c > index 4c8fcee..b8a4fb1 100644 > --- a/arch_init.c > +++ b/arch_init.c > @@ -603,6 +603,25 @@ static void migration_bitmap_sync(void) > } > } > > +void qmp_dump_pc_ram(const char *file, Error **errp) { > + > + int rc; > + int fd; > + fd = open(file, > + O_CREAT|O_WRONLY, > + S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); > + assert(-1 != fd); > + > + RAMBlock *block = QLIST_FIRST_RCU(&ram_list.blocks); > + uint64_t offset; > + for (offset=0; offset<ram_size; offset+=TARGET_PAGE_SIZE) { > + rc = write(fd, block->host+offset, TARGET_PAGE_SIZE); > + assert(TARGET_PAGE_SIZE == rc); > + } > + rc = close(fd); > + assert(0 == rc); > +} > + > /** > * ram_save_page: Send the given page to the stream > * > diff --git a/hmp-commands.hx b/hmp-commands.hx > index 3089533..0c47a4f 100644 > --- a/hmp-commands.hx > +++ b/hmp-commands.hx > @@ -1043,6 +1043,22 @@ gdb. Without -z|-l|-s, the dump format is ELF. > ETEXI > > { > + .name = "dump-pc-ram", > + .args_type = "filename:F", > + .params = "filename", > + .help = "dump pc ram to file", > + .mhandler.cmd = hmp_dump_pc_ram, > + }, > + > + > +STEXI > +@item dump-pc-ram > +@findex dump-guest-memory > +Dump pc ram to file. > + filename: dump file name > +ETEXI > + > + { > .name = "snapshot_blkdev", > .args_type = "reuse:-n,device:B,snapshot-file:s?,format:s?", > .params = "[-n] device [new-image-file] [format]", > diff --git a/hmp.c b/hmp.c > index f31ae27..5e27dd8 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -1473,6 +1473,15 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict) > g_free(prot); > } > > +void hmp_dump_pc_ram(Monitor *mon, const QDict *qdict) > +{ > + Error *errp = NULL; > + const char *file = qdict_get_str(qdict, "filename"); > + > + qmp_dump_pc_ram(file, &errp); > + hmp_handle_error(mon, &errp); > +} > + > void hmp_netdev_add(Monitor *mon, const QDict *qdict) > { > Error *err = NULL; > diff --git a/hmp.h b/hmp.h > index 2b9308b..805a71b 100644 > --- a/hmp.h > +++ b/hmp.h > @@ -79,6 +79,7 @@ void hmp_block_job_complete(Monitor *mon, const QDict *qdict); > void hmp_migrate(Monitor *mon, const QDict *qdict); > void hmp_device_del(Monitor *mon, const QDict *qdict); > void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict); > +void hmp_dump_pc_ram(Monitor *mon, const QDict *qdict); > void hmp_netdev_add(Monitor *mon, const QDict *qdict); > void hmp_netdev_del(Monitor *mon, const QDict *qdict); > void hmp_getfd(Monitor *mon, const QDict *qdict); > diff --git a/qapi-schema.json b/qapi-schema.json > index ac9594d..338bfd3 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -3648,3 +3648,14 @@ > # Since: 2.1 > ## > { 'command': 'rtc-reset-reinjection' } > + > +## > +# @dump-pc-ram: > +# > +# Checkpoints guest. > +# > +# @file: the file to save the memory to as binary data > +# > +# Returns: Nothing on success > +## > +{ 'command': 'dump-pc-ram', 'data': {'file': 'str'} } > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 3a42ad0..0be3c42 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -854,6 +854,25 @@ Notes: > EQMP > > { > + .name = "dump-pc-ram", > + .args_type = "file:s", > + .params = "file", > + .help = "dump pc ram to file", > + .user_print = monitor_user_noop, > + .mhandler.cmd_new = qmp_marshal_input_dump_pc_ram, > + }, > + > +SQMP > +dump > + > + > +Dump pc ram to file. > + > +Arguments: > + > +EQMP > + > + { > .name = "query-dump-guest-memory-capability", > .args_type = "", > .mhandler.cmd_new = qmp_marshal_input_query_dump_guest_memory_capability, > -- > 2.0.5 > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Hi David, thank you for the feedback! On 11/16/2015 05:46 PM, Dr. David Alan Gilbert wrote: > * Bohdan Trach (bv.trach@gmail.com) wrote: >> From: Bohdan Trach <bohdan.trach@mailbox.tu-dresden.de> >> >> dump-pc-mem command is added for checkpointing guest memory to >> file. Only system RAM region is saved. This checkpoint is later used to >> recover unchanged pages. > > Why not just use the 'dump_guest_memory' commands; they dump it in interesting > existing formats; they have headers in the files as well rather than just > a raw blob of data. > If you wanted to restrict to only certain RAM blocks, then I'd suggest adding > a feature to that existing command. > You might also find that you want other RAMBlocks as well, for example where > RAM is added using hot plu, those are separate RAM blocks. We will try to rework these patches to use existing formats. Current format was used because it is extremely simple to work with. The restriction of saving only 'pc.ram' RAMBlock is just the consequence of this design choice. > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK >
* Bohdan Trach (bohdan.trach@mailbox.tu-dresden.de) wrote: > Hi David, thank you for the feedback! No problem, sorry for the delay, > On 11/16/2015 05:46 PM, Dr. David Alan Gilbert wrote: > > * Bohdan Trach (bv.trach@gmail.com) wrote: > >> From: Bohdan Trach <bohdan.trach@mailbox.tu-dresden.de> > >> > >> dump-pc-mem command is added for checkpointing guest memory to > >> file. Only system RAM region is saved. This checkpoint is later used to > >> recover unchanged pages. > > > > Why not just use the 'dump_guest_memory' commands; they dump it in interesting > > existing formats; they have headers in the files as well rather than just > > a raw blob of data. > > If you wanted to restrict to only certain RAM blocks, then I'd suggest adding > > a feature to that existing command. > > You might also find that you want other RAMBlocks as well, for example where > > RAM is added using hot plu, those are separate RAM blocks. > > We will try to rework these patches to use existing formats. Current > format was used because it is extremely simple to work with. The > restriction of saving only 'pc.ram' RAMBlock is just the consequence > of this design choice. I think the ELF one should actually turn out to be the easiest; there's probably some existing ELF code you can find in the QEMU code base to make it easier to parse, but once you find the start of the block it should then be contiguous. Dave > > -- > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > > > > -- > With best regards, > Bohdan Trach -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff --git a/arch_init.c b/arch_init.c index 4c8fcee..b8a4fb1 100644 --- a/arch_init.c +++ b/arch_init.c @@ -603,6 +603,25 @@ static void migration_bitmap_sync(void) } } +void qmp_dump_pc_ram(const char *file, Error **errp) { + + int rc; + int fd; + fd = open(file, + O_CREAT|O_WRONLY, + S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); + assert(-1 != fd); + + RAMBlock *block = QLIST_FIRST_RCU(&ram_list.blocks); + uint64_t offset; + for (offset=0; offset<ram_size; offset+=TARGET_PAGE_SIZE) { + rc = write(fd, block->host+offset, TARGET_PAGE_SIZE); + assert(TARGET_PAGE_SIZE == rc); + } + rc = close(fd); + assert(0 == rc); +} + /** * ram_save_page: Send the given page to the stream * diff --git a/hmp-commands.hx b/hmp-commands.hx index 3089533..0c47a4f 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1043,6 +1043,22 @@ gdb. Without -z|-l|-s, the dump format is ELF. ETEXI { + .name = "dump-pc-ram", + .args_type = "filename:F", + .params = "filename", + .help = "dump pc ram to file", + .mhandler.cmd = hmp_dump_pc_ram, + }, + + +STEXI +@item dump-pc-ram +@findex dump-guest-memory +Dump pc ram to file. + filename: dump file name +ETEXI + + { .name = "snapshot_blkdev", .args_type = "reuse:-n,device:B,snapshot-file:s?,format:s?", .params = "[-n] device [new-image-file] [format]", diff --git a/hmp.c b/hmp.c index f31ae27..5e27dd8 100644 --- a/hmp.c +++ b/hmp.c @@ -1473,6 +1473,15 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict) g_free(prot); } +void hmp_dump_pc_ram(Monitor *mon, const QDict *qdict) +{ + Error *errp = NULL; + const char *file = qdict_get_str(qdict, "filename"); + + qmp_dump_pc_ram(file, &errp); + hmp_handle_error(mon, &errp); +} + void hmp_netdev_add(Monitor *mon, const QDict *qdict) { Error *err = NULL; diff --git a/hmp.h b/hmp.h index 2b9308b..805a71b 100644 --- a/hmp.h +++ b/hmp.h @@ -79,6 +79,7 @@ void hmp_block_job_complete(Monitor *mon, const QDict *qdict); void hmp_migrate(Monitor *mon, const QDict *qdict); void hmp_device_del(Monitor *mon, const QDict *qdict); void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict); +void hmp_dump_pc_ram(Monitor *mon, const QDict *qdict); void hmp_netdev_add(Monitor *mon, const QDict *qdict); void hmp_netdev_del(Monitor *mon, const QDict *qdict); void hmp_getfd(Monitor *mon, const QDict *qdict); diff --git a/qapi-schema.json b/qapi-schema.json index ac9594d..338bfd3 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -3648,3 +3648,14 @@ # Since: 2.1 ## { 'command': 'rtc-reset-reinjection' } + +## +# @dump-pc-ram: +# +# Checkpoints guest. +# +# @file: the file to save the memory to as binary data +# +# Returns: Nothing on success +## +{ 'command': 'dump-pc-ram', 'data': {'file': 'str'} } diff --git a/qmp-commands.hx b/qmp-commands.hx index 3a42ad0..0be3c42 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -854,6 +854,25 @@ Notes: EQMP { + .name = "dump-pc-ram", + .args_type = "file:s", + .params = "file", + .help = "dump pc ram to file", + .user_print = monitor_user_noop, + .mhandler.cmd_new = qmp_marshal_input_dump_pc_ram, + }, + +SQMP +dump + + +Dump pc ram to file. + +Arguments: + +EQMP + + { .name = "query-dump-guest-memory-capability", .args_type = "", .mhandler.cmd_new = qmp_marshal_input_query_dump_guest_memory_capability,