| Message ID | 1312376904-16115-28-git-send-email-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 08/03/2011 03:08 PM, Markus Armbruster wrote: > Breaks migration of qdevs "scsi-cd" and legacy "scsi-disk" to older > versions. We normally use subsections to avoid that. Not possible > here, because we don't have a section to begin with. Too bad. > > Signed-off-by: Markus Armbruster<armbru@redhat.com> > --- > hw/scsi-disk.c | 35 +++++++++++++++++++++++++++++++++++ > 1 files changed, 35 insertions(+), 0 deletions(-) > > diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c > index f223de6..04e0a77 100644 > --- a/hw/scsi-disk.c > +++ b/hw/scsi-disk.c > @@ -1301,6 +1301,39 @@ static int scsi_disk_initfn(SCSIDevice *dev) > return scsi_initfn(dev, scsi_type); > } > > +static int scsi_cd_post_load(void *opaque, int version_id) > +{ > + SCSIDiskState *s = opaque; > + > + bdrv_eject(s->bs, s->tray_open); > + bdrv_lock_medium(s->bs, s->tray_locked); > + return 0; > +} > + > +static const VMStateDescription vmstate_scsi_cd = { > + .name = "scsi-cd", > + .version_id = 0, > + .minimum_version_id = 0, > + .post_load = scsi_cd_post_load, > + .fields = (VMStateField[]) { > + VMSTATE_BOOL(tray_open, SCSIDiskState), > + VMSTATE_BOOL(tray_locked, SCSIDiskState), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static const VMStateDescription vmstate_scsi_disk = { > + .name = "scsi-disk", > + .version_id = 0, > + .minimum_version_id = 0, > + .post_load = scsi_cd_post_load, > + .fields = (VMStateField[]) { > + VMSTATE_BOOL(tray_open, SCSIDiskState), > + VMSTATE_BOOL(tray_locked, SCSIDiskState), > + VMSTATE_END_OF_LIST() > + } > +}; > + And again; if we had just one 'flags' value we would have to save only one variable. And wouldn't need to touch this one for future states. > #define DEFINE_SCSI_DISK_PROPERTIES() \ > DEFINE_BLOCK_PROPERTIES(SCSIDiskState, qdev.conf), \ > DEFINE_PROP_STRING("ver", SCSIDiskState, version), \ > @@ -1333,6 +1366,7 @@ static SCSIDeviceInfo scsi_disk_info[] = { > .qdev.fw_name = "disk", > .qdev.desc = "virtual SCSI CD-ROM", > .qdev.size = sizeof(SCSIDiskState), > + .qdev.vmsd =&vmstate_scsi_cd, > .qdev.reset = scsi_disk_reset, > .init = scsi_cd_initfn, > .destroy = scsi_destroy, > @@ -1353,6 +1387,7 @@ static SCSIDeviceInfo scsi_disk_info[] = { > .qdev.fw_name = "disk", > .qdev.desc = "virtual SCSI disk or CD-ROM (legacy)", > .qdev.size = sizeof(SCSIDiskState), > + .qdev.vmsd =&vmstate_scsi_disk, > .qdev.reset = scsi_disk_reset, > .init = scsi_disk_initfn, > .destroy = scsi_destroy, Cheers, Hannes
On 08/04/2011 08:23 AM, Hannes Reinecke wrote: > On 08/03/2011 03:08 PM, Markus Armbruster wrote: >> Breaks migration of qdevs "scsi-cd" and legacy "scsi-disk" to older >> versions. We normally use subsections to avoid that. Not possible >> here, because we don't have a section to begin with. Too bad. >> >> Signed-off-by: Markus Armbruster<armbru@redhat.com> >> --- >> hw/scsi-disk.c | 35 +++++++++++++++++++++++++++++++++++ >> 1 files changed, 35 insertions(+), 0 deletions(-) >> >> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c >> index f223de6..04e0a77 100644 >> --- a/hw/scsi-disk.c >> +++ b/hw/scsi-disk.c >> @@ -1301,6 +1301,39 @@ static int scsi_disk_initfn(SCSIDevice *dev) >> return scsi_initfn(dev, scsi_type); >> } >> >> +static int scsi_cd_post_load(void *opaque, int version_id) >> +{ >> + SCSIDiskState *s = opaque; >> + >> + bdrv_eject(s->bs, s->tray_open); >> + bdrv_lock_medium(s->bs, s->tray_locked); >> + return 0; >> +} >> + >> +static const VMStateDescription vmstate_scsi_cd = { >> + .name = "scsi-cd", >> + .version_id = 0, >> + .minimum_version_id = 0, >> + .post_load = scsi_cd_post_load, >> + .fields = (VMStateField[]) { >> + VMSTATE_BOOL(tray_open, SCSIDiskState), >> + VMSTATE_BOOL(tray_locked, SCSIDiskState), >> + VMSTATE_END_OF_LIST() >> + } >> +}; >> + >> +static const VMStateDescription vmstate_scsi_disk = { >> + .name = "scsi-disk", >> + .version_id = 0, >> + .minimum_version_id = 0, >> + .post_load = scsi_cd_post_load, >> + .fields = (VMStateField[]) { >> + VMSTATE_BOOL(tray_open, SCSIDiskState), >> + VMSTATE_BOOL(tray_locked, SCSIDiskState), >> + VMSTATE_END_OF_LIST() >> + } >> +}; >> + > And again; if we had just one 'flags' value we would have to save only > one variable. And wouldn't need to touch this one for future states. Agreed, but we have time until 0.16 to fix this. I plan to migrate more stuff (unit attention state, for one), I'll take care of this too. Paolo
Am 04.08.2011 08:23, schrieb Hannes Reinecke: > On 08/03/2011 03:08 PM, Markus Armbruster wrote: >> Breaks migration of qdevs "scsi-cd" and legacy "scsi-disk" to older >> versions. We normally use subsections to avoid that. Not possible >> here, because we don't have a section to begin with. Too bad. >> >> Signed-off-by: Markus Armbruster<armbru@redhat.com> >> --- >> hw/scsi-disk.c | 35 +++++++++++++++++++++++++++++++++++ >> 1 files changed, 35 insertions(+), 0 deletions(-) >> >> diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c >> index f223de6..04e0a77 100644 >> --- a/hw/scsi-disk.c >> +++ b/hw/scsi-disk.c >> @@ -1301,6 +1301,39 @@ static int scsi_disk_initfn(SCSIDevice *dev) >> return scsi_initfn(dev, scsi_type); >> } >> >> +static int scsi_cd_post_load(void *opaque, int version_id) >> +{ >> + SCSIDiskState *s = opaque; >> + >> + bdrv_eject(s->bs, s->tray_open); >> + bdrv_lock_medium(s->bs, s->tray_locked); >> + return 0; >> +} >> + >> +static const VMStateDescription vmstate_scsi_cd = { >> + .name = "scsi-cd", >> + .version_id = 0, >> + .minimum_version_id = 0, >> + .post_load = scsi_cd_post_load, >> + .fields = (VMStateField[]) { >> + VMSTATE_BOOL(tray_open, SCSIDiskState), >> + VMSTATE_BOOL(tray_locked, SCSIDiskState), >> + VMSTATE_END_OF_LIST() >> + } >> +}; >> + >> +static const VMStateDescription vmstate_scsi_disk = { >> + .name = "scsi-disk", >> + .version_id = 0, >> + .minimum_version_id = 0, >> + .post_load = scsi_cd_post_load, >> + .fields = (VMStateField[]) { >> + VMSTATE_BOOL(tray_open, SCSIDiskState), >> + VMSTATE_BOOL(tray_locked, SCSIDiskState), >> + VMSTATE_END_OF_LIST() >> + } >> +}; >> + > And again; if we had just one 'flags' value we would have to save > only one variable. And wouldn't need to touch this one for future > states. I think the policy is that you need to add a subsection anyway, even if you just add a flag to an existing field. Kevin
Am 03.08.2011 15:08, schrieb Markus Armbruster: > Breaks migration of qdevs "scsi-cd" and legacy "scsi-disk" to older > versions. We normally use subsections to avoid that. Not possible > here, because we don't have a section to begin with. Too bad. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> A more logical approach would be to add migration support to SCSI first. I guess adding .unmigratable = 1 makes more sense at the moment. Kevin
Kevin Wolf <kwolf@redhat.com> writes: > Am 03.08.2011 15:08, schrieb Markus Armbruster: >> Breaks migration of qdevs "scsi-cd" and legacy "scsi-disk" to older >> versions. We normally use subsections to avoid that. Not possible >> here, because we don't have a section to begin with. Too bad. >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> > > A more logical approach would be to add migration support to SCSI first. Isn't that what I do? > I guess adding .unmigratable = 1 makes more sense at the moment. Why?
Am 02.09.2011 17:35, schrieb Markus Armbruster: > Kevin Wolf <kwolf@redhat.com> writes: > >> Am 03.08.2011 15:08, schrieb Markus Armbruster: >>> Breaks migration of qdevs "scsi-cd" and legacy "scsi-disk" to older >>> versions. We normally use subsections to avoid that. Not possible >>> here, because we don't have a section to begin with. Too bad. >>> >>> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> >> A more logical approach would be to add migration support to SCSI first. > > Isn't that what I do? > >> I guess adding .unmigratable = 1 makes more sense at the moment. > > Why? Hm, you mean that we can successfully migrate SCSI today? SCSIDiskState doesn't seem to directly have anything variable that must be migrated. I think SCSIDevice does have fields that need to be migrated (unit_attention, sense and the request list). I'm not sure what other state scsi-bus involves, but unless the SCSI controllers can restore all of it, it should have a VMState, too. Kevin
Kevin Wolf <kwolf@redhat.com> writes: > Am 02.09.2011 17:35, schrieb Markus Armbruster: >> Kevin Wolf <kwolf@redhat.com> writes: >> >>> Am 03.08.2011 15:08, schrieb Markus Armbruster: >>>> Breaks migration of qdevs "scsi-cd" and legacy "scsi-disk" to older >>>> versions. We normally use subsections to avoid that. Not possible >>>> here, because we don't have a section to begin with. Too bad. >>>> >>>> Signed-off-by: Markus Armbruster <armbru@redhat.com> >>> >>> A more logical approach would be to add migration support to SCSI first. >> >> Isn't that what I do? >> >>> I guess adding .unmigratable = 1 makes more sense at the moment. >> >> Why? > > Hm, you mean that we can successfully migrate SCSI today? > > SCSIDiskState doesn't seem to directly have anything variable that must > be migrated. I think SCSIDevice does have fields that need to be > migrated (unit_attention, sense and the request list). I'm not sure what > other state scsi-bus involves, but unless the SCSI controllers can > restore all of it, it should have a VMState, too. Easiest solution is to just drop my patch then.
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index f223de6..04e0a77 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -1301,6 +1301,39 @@ static int scsi_disk_initfn(SCSIDevice *dev) return scsi_initfn(dev, scsi_type); } +static int scsi_cd_post_load(void *opaque, int version_id) +{ + SCSIDiskState *s = opaque; + + bdrv_eject(s->bs, s->tray_open); + bdrv_lock_medium(s->bs, s->tray_locked); + return 0; +} + +static const VMStateDescription vmstate_scsi_cd = { + .name = "scsi-cd", + .version_id = 0, + .minimum_version_id = 0, + .post_load = scsi_cd_post_load, + .fields = (VMStateField[]) { + VMSTATE_BOOL(tray_open, SCSIDiskState), + VMSTATE_BOOL(tray_locked, SCSIDiskState), + VMSTATE_END_OF_LIST() + } +}; + +static const VMStateDescription vmstate_scsi_disk = { + .name = "scsi-disk", + .version_id = 0, + .minimum_version_id = 0, + .post_load = scsi_cd_post_load, + .fields = (VMStateField[]) { + VMSTATE_BOOL(tray_open, SCSIDiskState), + VMSTATE_BOOL(tray_locked, SCSIDiskState), + VMSTATE_END_OF_LIST() + } +}; + #define DEFINE_SCSI_DISK_PROPERTIES() \ DEFINE_BLOCK_PROPERTIES(SCSIDiskState, qdev.conf), \ DEFINE_PROP_STRING("ver", SCSIDiskState, version), \ @@ -1333,6 +1366,7 @@ static SCSIDeviceInfo scsi_disk_info[] = { .qdev.fw_name = "disk", .qdev.desc = "virtual SCSI CD-ROM", .qdev.size = sizeof(SCSIDiskState), + .qdev.vmsd = &vmstate_scsi_cd, .qdev.reset = scsi_disk_reset, .init = scsi_cd_initfn, .destroy = scsi_destroy, @@ -1353,6 +1387,7 @@ static SCSIDeviceInfo scsi_disk_info[] = { .qdev.fw_name = "disk", .qdev.desc = "virtual SCSI disk or CD-ROM (legacy)", .qdev.size = sizeof(SCSIDiskState), + .qdev.vmsd = &vmstate_scsi_disk, .qdev.reset = scsi_disk_reset, .init = scsi_disk_initfn, .destroy = scsi_destroy,
Breaks migration of qdevs "scsi-cd" and legacy "scsi-disk" to older versions. We normally use subsections to avoid that. Not possible here, because we don't have a section to begin with. Too bad. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- hw/scsi-disk.c | 35 +++++++++++++++++++++++++++++++++++ 1 files changed, 35 insertions(+), 0 deletions(-)