Message ID | 31e7e4aefc644057186f501bfef4584d337a4d9c.1276607973.git.quintela@redhat.com |
---|---|
State | New |
Headers | show |
On 06/15/2010 04:31 PM, Juan Quintela wrote: > +static bool ide_bmdma_current_needed(void *opaque) > +{ > + BMDMAState *bm = opaque; > + > + return (bm->cur_prd_len != 0); > +} > + > +static const VMStateDescription vmstate_bmdma_current = { > + .name = "ide bmdma_current", > + .version_id = 1, > + .minimum_version_id = 1, > + .minimum_version_id_old = 1, > Can we allow these to default to 0? Most subsections (and most new sections) won't need version numbers. > + .fields = (VMStateField []) { > + VMSTATE_UINT32(cur_addr, BMDMAState), > + VMSTATE_UINT32(cur_prd_last, BMDMAState), > + VMSTATE_UINT32(cur_prd_addr, BMDMAState), > + VMSTATE_UINT32(cur_prd_len, BMDMAState), > + VMSTATE_END_OF_LIST() > + } > +}; > + > + > static const VMStateDescription vmstate_bmdma = { > .name = "ide bmdma", > .version_id = 3, > @@ -134,6 +156,14 @@ static const VMStateDescription vmstate_bmdma = { > VMSTATE_UINT32(nsector, BMDMAState), > VMSTATE_UINT8(unit, BMDMAState), > VMSTATE_END_OF_LIST() > + }, > + .subsections = (VMStateSubsection []) { > + { > + .vmsd =&vmstate_bmdma_current, > + .needed = ide_bmdma_current_needed, > + }, { > + /* empty */ > + } > } > }; > Looks concise and simple.
Avi Kivity <avi@redhat.com> wrote: > On 06/15/2010 04:31 PM, Juan Quintela wrote: >> +static bool ide_bmdma_current_needed(void *opaque) >> +{ >> + BMDMAState *bm = opaque; >> + >> + return (bm->cur_prd_len != 0); >> +} >> + >> +static const VMStateDescription vmstate_bmdma_current = { >> + .name = "ide bmdma_current", >> + .version_id = 1, >> + .minimum_version_id = 1, >> + .minimum_version_id_old = 1, >> > > Can we allow these to default to 0? Most subsections (and most new > sections) won't need version numbers. It is not clear for all if default value is 0 or 1. (not consistent). We can put it as 0. >> + .fields = (VMStateField []) { >> + VMSTATE_UINT32(cur_addr, BMDMAState), >> + VMSTATE_UINT32(cur_prd_last, BMDMAState), >> + VMSTATE_UINT32(cur_prd_addr, BMDMAState), >> + VMSTATE_UINT32(cur_prd_len, BMDMAState), >> + VMSTATE_END_OF_LIST() >> + } >> +}; >> + >> + >> static const VMStateDescription vmstate_bmdma = { >> .name = "ide bmdma", >> .version_id = 3, >> @@ -134,6 +156,14 @@ static const VMStateDescription vmstate_bmdma = { >> VMSTATE_UINT32(nsector, BMDMAState), >> VMSTATE_UINT8(unit, BMDMAState), >> VMSTATE_END_OF_LIST() >> + }, >> + .subsections = (VMStateSubsection []) { >> + { >> + .vmsd =&vmstate_bmdma_current, >> + .needed = ide_bmdma_current_needed, >> + }, { >> + /* empty */ >> + } >> } >> }; >> > > Looks concise and simple.
On 06/15/2010 03:31 PM, Juan Quintela wrote:
> Signed-off-by: Juan Quintela<quintela@redhat.com>
Sorry if this has been discussed to death before (if so I must have
missed it...).
With subsections available, what about taking advantage of the new
protocol extension and add to the subsection info about the size of the
subsection?
Also, with the size information, would it make sense to specify optional
subsections that the receiver could choose to ignore? Mandatory
subsections are something such that round-trip A->B->A fail unless B
understands the subsection, while optional subsections are such that A
can provide a default. IDE subsections would be optional, for example.
Paolo
Paolo Bonzini <pbonzini@redhat.com> wrote: > On 06/15/2010 03:31 PM, Juan Quintela wrote: >> Signed-off-by: Juan Quintela<quintela@redhat.com> > > Sorry if this has been discussed to death before (if so I must have > missed it...). > > With subsections available, what about taking advantage of the new > protocol extension and add to the subsection info about the size of > the subsection? Not trivial with current infrastructure :( > Also, with the size information, would it make sense to specify > optional subsections that the receiver could choose to ignore? We agreed that this was going to be forbidden. If sender send data -> it needs to be received. Sender can decide to not send a subsection if its data is not needed. > Mandatory subsections are something such that round-trip A->B->A fail > unless B understands the subsection, while optional subsections are > such that A can provide a default. IDE subsections would be optional, > for example. This is the whole point of the .needed() function. if neeeded(foo_subsection) returns false -> subsection is not needed. If it returns true -> it is needed, destination has to understand it. Later, Juan.
diff --git a/hw/ide/pci.c b/hw/ide/pci.c index 780fc5f..4331d77 100644 --- a/hw/ide/pci.c +++ b/hw/ide/pci.c @@ -121,6 +121,28 @@ void bmdma_addr_writel(void *opaque, uint32_t addr, uint32_t val) bm->cur_addr = bm->addr; } +static bool ide_bmdma_current_needed(void *opaque) +{ + BMDMAState *bm = opaque; + + return (bm->cur_prd_len != 0); +} + +static const VMStateDescription vmstate_bmdma_current = { + .name = "ide bmdma_current", + .version_id = 1, + .minimum_version_id = 1, + .minimum_version_id_old = 1, + .fields = (VMStateField []) { + VMSTATE_UINT32(cur_addr, BMDMAState), + VMSTATE_UINT32(cur_prd_last, BMDMAState), + VMSTATE_UINT32(cur_prd_addr, BMDMAState), + VMSTATE_UINT32(cur_prd_len, BMDMAState), + VMSTATE_END_OF_LIST() + } +}; + + static const VMStateDescription vmstate_bmdma = { .name = "ide bmdma", .version_id = 3, @@ -134,6 +156,14 @@ static const VMStateDescription vmstate_bmdma = { VMSTATE_UINT32(nsector, BMDMAState), VMSTATE_UINT8(unit, BMDMAState), VMSTATE_END_OF_LIST() + }, + .subsections = (VMStateSubsection []) { + { + .vmsd = &vmstate_bmdma_current, + .needed = ide_bmdma_current_needed, + }, { + /* empty */ + } } };
Signed-off-by: Juan Quintela <quintela@redhat.com> --- hw/ide/pci.c | 30 ++++++++++++++++++++++++++++++ 1 files changed, 30 insertions(+), 0 deletions(-)