| Message ID | cover.1276607973.git.quintela@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 06/15/2010 08:31 AM, Juan Quintela wrote: > Hi > > At the end, here is the migration subsections implementation. As an example I ported the last > two ide changes to migration to work with subsections. Notes: > > - subsections > I went for qemu_peek_byte() insteadof adding a subsection part in > qemu_loadvm_state() due to two reasons: > - it makes mandatory that subsections came after sections (better for error messages) > - it makes post_load() for the section to be run after subsections are loaded. > I think that running section post_load() and then subsections can make for some subtle > errors. > How does it works? > We have a new array of subsections at the end of each section (it can be NULL). > Each subsection is composed of VMStateDescription and a test function. test function > checks if subsection is needed or not. if needed, it is just emmited. > On load, we peek to see if after a section is loaded, if there is any subsection > at the end, and if so, we search for it on this section subsections. > > > - ide: 1st revert is not clear because there has been posterior changes that I honored. > only change done is that ide_dummy_transfer_stop to transfer_end_table for it to be > complete. > > - testing. In normal operation this code is not triggered (one of the reason for not wanting to > sent it in the 1st place). I used patch attached at the end to trigger it. > > Commetnts? > Obviously, we can't change IDE like this but the implementation looks pretty sane. Can you add something to docs/ that explains how this works? My only concern is that the code doesn't make it obvious what a subsection is, when to use it, and why it's safe from an implementation perspective. Very nice solution though. Regards, Anthony Liguori > Later, Juan. > > > Juan Quintela (5): > Revert "ide save/restore pio/atapi cmd transfer fields and io buffer" > Revert "ide save/restore current transfer fields" > vmstate: add subsections code > ide: fix migration in the middle of pio operation > ide: fix migration in the middle of a bmdma transfer > > hw/hw.h | 6 ++++ > hw/ide/core.c | 72 ++++++++++++++++++++++++++++++++------------- > hw/ide/pci.c | 38 ++++++++++++++++++++---- > savevm.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++- > 4 files changed, 178 insertions(+), 28 deletions(-) > > > diff --git a/hw/ide/core.c b/hw/ide/core.c > index 59341a1..a4e6b82 100644 > --- a/hw/ide/core.c > +++ b/hw/ide/core.c > @@ -421,6 +421,15 @@ static void ide_sector_read(IDEState *s) > ide_set_irq(s->bus); > ide_set_sector(s, sector_num + n); > s->nsector -= n; > + if ((s->status& DRQ_STAT)) { > + static int val = 1; > + if (((val++) == 1000)) { > + qemu_aio_flush(); > + vm_stop(0); > + printf("stopped %d ide_ioport_readaaa\n", val); > + } > + } > + > } > } > > @@ -2744,6 +2753,14 @@ static int ide_drive_pio_post_load(void *opaque, int version_id) > s->data_ptr = s->io_buffer + s->cur_io_buffer_offset; > s->data_end = s->data_ptr + s->cur_io_buffer_len; > > + printf("addr %p: status %d\n", s, s->status& DRQ_STAT); > + printf("\tdata_ptr %p\n", s->data_ptr); > + printf("\tdata_end %p\n", s->data_end); > + printf("\tio_buffer %p\n", s->io_buffer); > + printf("\treq_nb_sectors %d\n", s->req_nb_sectors); > + printf("\tidx %d\n", transfer_end_table_idx(s->end_transfer_func)); > + printf("\telementary_transfer_size %d\n", s->elementary_transfer_size); > + printf("\tpacket_transfer_size %d\n", s->packet_transfer_size); > return 0; > } > > @@ -2763,6 +2780,15 @@ static void ide_drive_pio_pre_save(void *opaque) > } else { > s->end_transfer_fn_idx = idx; > } > + > + printf("addr %p: status %d\n", s, s->status& DRQ_STAT); > + printf("\tdata_ptr %p\n", s->data_ptr); > + printf("\tdata_end %p\n", s->data_end); > + printf("\tio_buffer %p\n", s->io_buffer); > + printf("\treq_nb_sectors %d\n", s->req_nb_sectors); > + printf("\tidx %d\n", transfer_end_table_idx(s->end_transfer_func)); > + printf("\telementary_transfer_size %d\n", s->elementary_transfer_size); > + printf("\tpacket_transfer_size %d\n", s->packet_transfer_size); > } > > static bool ide_drive_pio_state_needed(void *opaque) > >
Anthony Liguori <anthony@codemonkey.ws> wrote: > On 06/15/2010 08:31 AM, Juan Quintela wrote: >> Hi >> >> At the end, here is the migration subsections implementation. As an example I ported the last >> two ide changes to migration to work with subsections. Notes: >> >> - subsections >> I went for qemu_peek_byte() insteadof adding a subsection part in >> qemu_loadvm_state() due to two reasons: >> - it makes mandatory that subsections came after sections (better for error messages) >> - it makes post_load() for the section to be run after subsections are loaded. >> I think that running section post_load() and then subsections can make for some subtle >> errors. >> How does it works? >> We have a new array of subsections at the end of each section (it can be NULL). >> Each subsection is composed of VMStateDescription and a test function. test function >> checks if subsection is needed or not. if needed, it is just emmited. >> On load, we peek to see if after a section is loaded, if there is any subsection >> at the end, and if so, we search for it on this section subsections. >> >> >> - ide: 1st revert is not clear because there has been posterior changes that I honored. >> only change done is that ide_dummy_transfer_stop to transfer_end_table for it to be >> complete. >> >> - testing. In normal operation this code is not triggered (one of the reason for not wanting to >> sent it in the 1st place). I used patch attached at the end to trigger it. >> >> Commetnts? >> > > Obviously, we can't change IDE like this but the implementation looks > pretty sane. Why? nothing has been released with 0.12.[34], and that is what broke migration in the 1st place. > Can you add something to docs/ that explains how this > works? My only concern is that the code doesn't make it obvious what > a subsection is, when to use it, and why it's safe from an > implementation perspective. Agreed, wanted to see if I got some comments 1st. Will resend tomorrow with some documentantion. > Very nice solution though. Thanks. Later, Juan.
diff --git a/hw/ide/core.c b/hw/ide/core.c index 59341a1..a4e6b82 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -421,6 +421,15 @@ static void ide_sector_read(IDEState *s) ide_set_irq(s->bus); ide_set_sector(s, sector_num + n); s->nsector -= n; + if ((s->status & DRQ_STAT)) { + static int val = 1; + if (((val++) == 1000)) { + qemu_aio_flush(); + vm_stop(0); + printf("stopped %d ide_ioport_readaaa\n", val); + } + } + } } @@ -2744,6 +2753,14 @@ static int ide_drive_pio_post_load(void *opaque, int version_id) s->data_ptr = s->io_buffer + s->cur_io_buffer_offset; s->data_end = s->data_ptr + s->cur_io_buffer_len; + printf("addr %p: status %d\n", s, s->status& DRQ_STAT); + printf("\tdata_ptr %p\n", s->data_ptr); + printf("\tdata_end %p\n", s->data_end); + printf("\tio_buffer %p\n", s->io_buffer); + printf("\treq_nb_sectors %d\n", s->req_nb_sectors); + printf("\tidx %d\n", transfer_end_table_idx(s->end_transfer_func)); + printf("\telementary_transfer_size %d\n", s->elementary_transfer_size); + printf("\tpacket_transfer_size %d\n", s->packet_transfer_size); return 0; } @@ -2763,6 +2780,15 @@ static void ide_drive_pio_pre_save(void *opaque) } else { s->end_transfer_fn_idx = idx; } + + printf("addr %p: status %d\n", s, s->status& DRQ_STAT); + printf("\tdata_ptr %p\n", s->data_ptr); + printf("\tdata_end %p\n", s->data_end); + printf("\tio_buffer %p\n", s->io_buffer); + printf("\treq_nb_sectors %d\n", s->req_nb_sectors); + printf("\tidx %d\n", transfer_end_table_idx(s->end_transfer_func)); + printf("\telementary_transfer_size %d\n", s->elementary_transfer_size); + printf("\tpacket_transfer_size %d\n", s->packet_transfer_size); } static bool ide_drive_pio_state_needed(void *opaque)
Hi At the end, here is the migration subsections implementation. As an example I ported the last two ide changes to migration to work with subsections. Notes: - subsections I went for qemu_peek_byte() insteadof adding a subsection part in qemu_loadvm_state() due to two reasons: - it makes mandatory that subsections came after sections (better for error messages) - it makes post_load() for the section to be run after subsections are loaded. I think that running section post_load() and then subsections can make for some subtle errors. How does it works? We have a new array of subsections at the end of each section (it can be NULL). Each subsection is composed of VMStateDescription and a test function. test function checks if subsection is needed or not. if needed, it is just emmited. On load, we peek to see if after a section is loaded, if there is any subsection at the end, and if so, we search for it on this section subsections. - ide: 1st revert is not clear because there has been posterior changes that I honored. only change done is that ide_dummy_transfer_stop to transfer_end_table for it to be complete. - testing. In normal operation this code is not triggered (one of the reason for not wanting to sent it in the 1st place). I used patch attached at the end to trigger it. Commetnts? Later, Juan. Juan Quintela (5): Revert "ide save/restore pio/atapi cmd transfer fields and io buffer" Revert "ide save/restore current transfer fields" vmstate: add subsections code ide: fix migration in the middle of pio operation ide: fix migration in the middle of a bmdma transfer hw/hw.h | 6 ++++ hw/ide/core.c | 72 ++++++++++++++++++++++++++++++++------------- hw/ide/pci.c | 38 ++++++++++++++++++++---- savevm.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 178 insertions(+), 28 deletions(-)