atapi: Implement 'media' subcommand for GESN
v2 cover atapi: Implement 'media' subcommand for GESN
The GET_EVENT_STATUS_NOTIFICATION ATAPI command is listed as a mandatory command in the spec but we don't really implement it any of its sub-commands.
The commit message for the last commit explains why implementing just the media subcommand is helpful and how it goes a long way in getting guests to behave as expected.
The difference from the RFC series sent earlier is:
- Split into more patches
- Add tray open/close notification (from Markus)
There certainly is much more work to be done for the other commands and also for state change handling (tray open / close / new media) overall for the block layer, but this is a good first step in being spec-compliant and at the same time making guests work.
v2:
- Update comments
- Use struct instead of enum for cdb packet
- Add a new subsection to vmstate for new fields for save/restore
v1:
- Split into more patches
- Add tray open/close notification (from Markus)
RFC:
- Orig. series
Amit Shah (6):
atapi: Allow GET_EVENT_STATUS_NOTIFICATION after media change
atapi: Move GET_EVENT_STATUS_NOTIFICATION command handling to its own
function
atapi: GESN: Spin off No Event Available handling into own function
atapi: GESN: Use structs for commonly-used field types
atapi: GESN: implement 'media' subcommand
atapi: Save / load the new GESN event states during migration
hw/ide/core.c | 186 +++++++++++++++++++++++++++++++++++++++++++++++------ hw/ide/internal.h | 6 ++ 2 files changed, 173 insertions(+), 19 deletions(-)
Signature · 1 line
1.7.4.2
PATCH v2 1/6 atapi: Allow GESN after media change
After a media change, the only commands allowed from the guest were REQUEST_SENSE and INQUIRY. The guest may also issue GET_EVENT_STATUS_NOTIFICATION commands to get media changed notification.
--- hw/ide/core.c | 19 +++++++++++++------ 1 files changed, 13 insertions(+), 6 deletions(-)
hw/ide/core.c +13 -6
@@ -1102,13 +1102,20 @@ static void ide_atapi_cmd(IDEState *s) printf("\n"); }#endif -/* If there's a UNIT_ATTENTION condition pending, only -REQUEST_SENSE and INQUIRY commands are allowed to complete. */ +/* +* If there's a UNIT_ATTENTION condition pending, only +* REQUEST_SENSE, INQUIRY and GET_EVENT_STATUS_NOTIFICATION +* commands are allowed to complete. MMC-5, section 4.1.6.1 lists +* only these commands being allowed to complete, with other +* commands getting a CHECK condition response unless a higher +* priority status, defined by the drive here, is pending. +*/ if (s->sense_key ==SENSE_UNIT_ATTENTION && - s->io_buffer[0 ] !=GPCMD_REQUEST_SENSE && - s->io_buffer[0 ] !=GPCMD_INQUIRY ) { -ide_atapi_cmd_check_status (s); -return ; + s->io_buffer[0 ] !=GPCMD_REQUEST_SENSE && + s->io_buffer[0 ] !=GPCMD_INQUIRY && + s->io_buffer[0 ] !=GPCMD_GET_EVENT_STATUS_NOTIFICATION ) { +ide_atapi_cmd_check_status (s); +return ; }switch (s->io_buffer[0 ]) {case GPCMD_TEST_UNIT_READY :
Signature · 1 line
1.7.4.2
Re: [Qemu-devel] [PATCH v2 1/6] atapi: Allow GESN after media change
Am 12.04.2011 11:27, schrieb Amit Shah:
Quoted 18 lines
After a media change, the only commands allowed from the guest were
REQUEST_SENSEand INQUIRY. The guest may also issueGET_EVENT_STATUS_NOTIFICATIONcommands to get media changed notification.Signed-off-by Amit Shah <[email protected]>--- hw/ide/core.c | 19 +++++++++++++------ 1 files changed, 13 insertions(+), 6 deletions(-)hw/ide/core.c +0
-0@@ -1102,13 +1102,20 @@ static void ide_atapi_cmd(IDEState *s) printf("\n"); }#endif
hw/ide/core.c static void ide_atapi_cmd(IDEState *s) · line 1112
Quoted 10 lines
hw/ide/core.c +8
-2-/* If there's a UNIT_ATTENTION condition pending, only -REQUEST_SENSE and INQUIRY commands are allowed to complete. */ +/* +* If there's a UNIT_ATTENTION condition pending, only +* REQUEST_SENSE, INQUIRY and GET_EVENT_STATUS_NOTIFICATION +* commands are allowed to complete. MMC-5, section 4.1.6.1 lists +* only these commands being allowed to complete, with other +* commands getting a CHECK condition response unless a higher +* priority status, defined by the drive here, is pending. +*/
GET CONFIGURATION is allowed as well. That would be an unrelated fix, though, so we can do it on top.
Kevin
PATCH v2 2/6 atapi: Move GET_EVENT_STATUS_NOTIFICATION command handling to its own function
In reply to Amit ShahThis makes the code more readable.
Also, there's a block like:
if () {
...
} else {
...
}
Split that into
if () {
...
return;
}
...
--- hw/ide/core.c | 37 ++++++++++++++++++++++++------------- 1 files changed, 24 insertions(+), 13 deletions(-)
hw/ide/core.c +24 -13
@@ -1084,6 +1084,29 @@ static int ide_dvd_read_structure(IDEState *s, int format, } } +static void handle_get_event_status_notification (IDEState *s, +uint8_t *buf, +const uint8_t *packet) +{ +unsigned int max_len; + + max_len =ube16_to_cpu (packet +7 ); + +if (!(packet[1 ] &0x01 )) {/* asynchronous mode */ +/* Only polling is supported, asynchronous mode is not. */ +ide_atapi_cmd_error (s,SENSE_ILLEGAL_REQUEST , +ASC_INV_FIELD_IN_CMD_PACKET ); +return ; + } + +/* polling */ +/* We don't support any event class (yet). */ +cpu_to_ube16 (buf,0x00 );/* No event descriptor returned */ + buf[2 ] =0x80 ;/* No Event Available (NEA) */ + buf[3 ] =0x00 ;/* Empty supported event classes */ +ide_atapi_cmd_reply (s,4 , max_len); +} +static void ide_atapi_cmd (IDEState *s) {const uint8_t *packet;
@@ -1529,19 +1552,7 @@ static void ide_atapi_cmd(IDEState *s)break ; }case GPCMD_GET_EVENT_STATUS_NOTIFICATION : - max_len =ube16_to_cpu (packet +7 ); - -if (packet[1 ] &0x01 ) {/* polling */ -/* We don't support any event class (yet). */ -cpu_to_ube16 (buf,0x00 );/* No event descriptor returned */ - buf[2 ] =0x80 ;/* No Event Available (NEA) */ - buf[3 ] =0x00 ;/* Empty supported event classes */ -ide_atapi_cmd_reply (s,4 , max_len); - }else {/* asynchronous mode */ -/* Only polling is supported, asynchronous mode is not. */ -ide_atapi_cmd_error (s,SENSE_ILLEGAL_REQUEST , -ASC_INV_FIELD_IN_CMD_PACKET ); - } +handle_get_event_status_notification (s, buf, packet);break ;default : ide_atapi_cmd_error(s, SENSE_ILLEGAL_REQUEST,
Signature · 1 line
1.7.4.2
PATCH v2 3/6 atapi: GESN: Spin off No Event Available handling into own function
In reply to Amit ShahHandle GET_EVENT_STATUS_NOTIFICATION's No Event Available status in its own function.
--- hw/ide/core.c | 21 +++++++++++++++------ 1 files changed, 15 insertions(+), 6 deletions(-)
hw/ide/core.c +15 -6
@@ -1084,14 +1084,24 @@ static int ide_dvd_read_structure(IDEState *s, int format, } } +static unsigned int event_status_nea (uint8_t *buf,unsigned int max_len) +{ +cpu_to_ube16 (buf,0x00 );/* No event descriptor returned */ + buf[2 ] =0x80 ;/* No Event Available (NEA) */ + buf[3 ] =0x00 ;/* Empty supported event classes */ + +return 4 ;/* We wrote to 4 bytes */ +} +static void handle_get_event_status_notification (IDEState *s,uint8_t *buf,const uint8_t *packet) { -unsigned int max_len; +unsigned int max_len, used_len; max_len =ube16_to_cpu (packet +7 ); +/* It is fine by the MMC spec to not support async mode operations */ if (!(packet[1 ] &0x01 )) {/* asynchronous mode */ /* Only polling is supported, asynchronous mode is not. */ ide_atapi_cmd_error(s, SENSE_ILLEGAL_REQUEST,
@@ -1099,12 +1109,11 @@ static void handle_get_event_status_notification(IDEState *s,return ; } -/* polling */ +/* polling mode operation */ +/* We don't support any event class (yet). */ -cpu_to_ube16 (buf,0x00 );/* No event descriptor returned */ - buf[2 ] =0x80 ;/* No Event Available (NEA) */ - buf[3 ] =0x00 ;/* Empty supported event classes */ -ide_atapi_cmd_reply (s,4 , max_len); + used_len =event_status_nea (buf, max_len); +ide_atapi_cmd_reply (s, used_len, max_len); }static void ide_atapi_cmd (IDEState *s)
Signature · 1 line
1.7.4.2
PATCH v2 4/6 atapi: GESN: Use structs for commonly-used field types
In reply to Amit ShahInstead of using magic numbers, use structs that are more descriptive of the fields being used.
--- hw/ide/core.c | 15 +++++++++++++-- 1 files changed, 13 insertions(+), 2 deletions(-)
hw/ide/core.c +13 -2
@@ -1097,12 +1097,23 @@ static void handle_get_event_status_notification(IDEState *s, uint8_t *buf, const uint8_t *packet) { +struct { +uint8_t opcode; +uint8_t polled;/* lsb bit is polled; others are reserved */ +uint8_t reserved2[2 ]; +uint8_t request; +uint8_t reserved3[2 ]; +uint8_t len_msb; +uint8_t len_lsb; +uint8_t control; + } __attribute__((packed)) *gesn_cdb;unsigned int max_len, used_len; - max_len =ube16_to_cpu (packet +7 ); + gesn_cdb = (void *)packet; + max_len =ube16_to_cpu (&gesn_cdb->len_msb);/* It is fine by the MMC spec to not support async mode operations */ -if (!(packet[1 ] &0x01 )) {/* asynchronous mode */ +if (!(gesn_cdb->polled &0x01 )) {/* asynchronous mode */ /* Only polling is supported, asynchronous mode is not. */ ide_atapi_cmd_error (s,SENSE_ILLEGAL_REQUEST ,ASC_INV_FIELD_IN_CMD_PACKET );
Signature · 1 line
1.7.4.2
Re: [Qemu-devel] [PATCH v2 4/6] atapi: GESN: Use structs for commonly-used field types
Am 12.04.2011 11:27, schrieb Amit Shah:
Quoted 13 lines
Instead of using magic numbers, use structs that are more descriptive of the fields being used.
Signed-off-by Amit Shah <[email protected]>--- hw/ide/core.c | 15 +++++++++++++-- 1 files changed, 13 insertions(+), 2 deletions(-)hw/ide/core.c +0
-0
hw/ide/core.c static void handle_get_event_status_notification(IDEState *s, · line 1104
Quoted 8 lines
hw/ide/core.c +5
-0@@ -1097,12 +1097,23 @@ static void handle_get_event_status_notification(IDEState *s, uint8_t *buf, const uint8_t *packet) { +struct { +uint8_t opcode; +uint8_t polled;/* lsb bit is polled; others are reserved */ +uint8_t reserved2[2 ]; +uint8_t request;
Maybe class would be a better name?
hw/ide/core.c static void handle_get_event_status_notification(IDEState *s, · line 1107
Quoted 3 lines
hw/ide/core.c +3
-0+uint8_t reserved3[2 ]; +uint8_t len_msb; +uint8_t len_lsb;
Why not a uint16_t length?
hw/ide/core.c static void handle_get_event_status_notification(IDEState *s, · line 1113
Quoted 7 lines
hw/ide/core.c +4
-1+uint8_t control; + } __attribute__((packed)) *gesn_cdb;unsigned int max_len, used_len; - max_len =ube16_to_cpu (packet +7 ); + gesn_cdb = (void *)packet; + max_len =ube16_to_cpu (&gesn_cdb->len_msb);
Instead of reading 16 bit from a 8 bit pointer, you could then use be16_to_cpu(gesn_cdb->length);
hw/ide/core.c static void handle_get_event_status_notification(IDEState *s, · line 1119
Quoted 7 lines
hw/ide/core.c +1
-1/* It is fine by the MMC spec to not support async mode operations */ -if (!(packet[1 ] &0x01 )) {/* asynchronous mode */ +if (!(gesn_cdb->polled &0x01 )) {/* asynchronous mode */ /* Only polling is supported, asynchronous mode is not. */ ide_atapi_cmd_error (s,SENSE_ILLEGAL_REQUEST ,ASC_INV_FIELD_IN_CMD_PACKET );
Kevin
PATCH v2 5/6 atapi: GESN: implement 'media' subcommand
In reply to Amit ShahImplement the 'media' sub-command of the GET_EVENT_STATUS_NOTIFICATION command. This helps us report tray open, tray closed, no media, media present states to the guest.
Newer Linux kernels (2.6.38+) rely on this command to revalidate discs after media change.
This patch also sends out tray open/closed status to the guest driver when requested e.g. via the CDROM_DRIVE_STATUS ioctl (thanks Markus). Without such notification, the guest and qemu's tray open/close status was frequently out of sync, causing installers like Anaconda detecting no disc instead of tray open, confusing them terribly.
--- hw/ide/core.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++- hw/ide/internal.h | 6 +++ 2 files changed, 96 insertions(+), 2 deletions(-)
hw/ide/core.c +90 -2
@@ -1084,6 +1084,47 @@ static int ide_dvd_read_structure(IDEState *s, int format, } } +static unsigned int event_status_media (IDEState *s, +uint8_t *buf, +unsigned int max_len, +unsigned int event_class, +unsigned int supported_events) +{ +enum media_event_code { + no_change =0 ,/* Status unchanged */ + eject_requested,/* received a request from user to eject */ + new_media,/* new media inserted and ready for access */ + media_removal,/* only for media changers */ + media_changed,/* only for media changers */ + bg_format_completed,/* MRW or DVD+RW b/g format completed */ + bg_format_restarted,/* MRW or DVD+RW b/g format restarted */ + }; +enum media_status { + tray_open =1 , + media_present =2 , + }; +uint8_t event_code, media_status; + + media_status =0 ; +if (s->bs->tray_open) { + media_status = tray_open; + }else if (bdrv_is_inserted (s->bs)) { + media_status = media_present; + } + +/* Event notification descriptor */ + event_code = no_change; +if (media_status != tray_open && s->events.new_media) { + event_code = new_media; + s->events.new_media = false; + } + + buf[4 ] = event_code; + buf[5 ] = media_status; + +return 6 ;/* We wrote to just 2 extra bytes from the header */ +} +static unsigned int event_status_nea (uint8_t *buf,unsigned int max_len) {cpu_to_ube16 (buf,0x00 );/* No event descriptor returned */
@@ -1107,7 +1148,28 @@ static void handle_get_event_status_notification(IDEState *s,uint8_t len_lsb;uint8_t control; } __attribute__((packed)) *gesn_cdb; +enum notification_class_request_type { + reserved1 =1 <<0 , + operational_change =1 <<1 , + power_management =1 <<2 , + external_request =1 <<3 , + media =1 <<4 , + multi_host =1 <<5 , + device_busy =1 <<6 , + reserved2 =1 <<7 , + }; +enum event_notification_class_field { + enc_no_events =0 , + enc_operational_change, + enc_power_management, + enc_external_request, + enc_media, + enc_multiple_hosts, + enc_device_busy, + enc_reserved, + };unsigned int max_len, used_len; +unsigned int supported_events; gesn_cdb = (void *)packet; max_len =ube16_to_cpu (&gesn_cdb->len_msb);
@@ -1122,8 +1184,33 @@ static void handle_get_event_status_notification(IDEState *s,/* polling mode operation */ -/* We don't support any event class (yet). */ - used_len =event_status_nea (buf, max_len); +/* +* These are the supported events. +* +* We currently only support requests of the 'media' type. +*/ + supported_events = media; + +/* +* Event notification header; will be overwritten by the +* NO_EVENT_AVAILABLE code if we don't have events: according to +* MMC-5 6.7.2.2, if nea = 1, event class field should be 0. +*/ +cpu_to_ube16 (buf, max_len); + buf[2 ] = event_class; + buf[3 ] = supported_events; + +/* +* Responses to requests are to be based on request priority. The +* notification_class_request_type enum above specifies the +* priority: upper elements are higher prio than lower ones. +*/ +if (gesn_cdb->request & media) { + used_len =event_status_media (s, buf, max_len, enc_media, + supported_events); + }else { + used_len =event_status_nea (buf, max_len); + }ide_atapi_cmd_reply (s, used_len, max_len); }
@@ -1650,6 +1737,7 @@ static void cdrom_change_cb(void *opaque, int reason) s->sense_key =SENSE_UNIT_ATTENTION ; s->asc =ASC_MEDIUM_MAY_HAVE_CHANGED ; s->cdrom_changed =1 ; + s->events.new_media = true;ide_set_irq (s->bus); }
hw/ide/internal.h +6 -0
@@ -373,6 +373,11 @@ typedef int DMAFunc(IDEDMA *);typedef int DMAIntFunc (IDEDMA *,int );typedef void DMARestartFunc (void *,int ,int ); +struct unreported_events { +bool eject_request; +bool new_media; +}; +/* NOTE: IDEState represents in fact one drive */ struct IDEState {IDEBus *bus;
@@ -408,6 +413,7 @@ struct IDEState {BlockDriverState *bs;char version[9 ];/* ATAPI specific */ +struct unreported_events events;uint8_t sense_key;uint8_t asc;uint8_t cdrom_changed;
Signature · 1 line
1.7.4.2
Re: [Qemu-devel] [PATCH v2 5/6] atapi: GESN: implement 'media' subcommand
Am 12.04.2011 11:27, schrieb Amit Shah:
Quoted 56 lines
Implement the 'media' sub-command of the
GET_EVENT_STATUS_NOTIFICATIONcommand. This helps us report tray open, tray closed, no media, media present states to the guest.Newer Linux kernels (2.6.38+) rely on this command to revalidate discs after media change.
This patch also sends out tray open/closed status to the guest driver when requested e.g. via the
CDROM_DRIVE_STATUSioctl (thanks Markus). Without such notification, the guest and qemu's tray open/close status was frequently out of sync, causing installers like Anaconda detecting no disc instead of tray open, confusing them terribly.Signed-off-by Amit Shah <[email protected]>--- hw/ide/core.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++- hw/ide/internal.h | 6 +++ 2 files changed, 96 insertions(+), 2 deletions(-)hw/ide/core.c +29
-0@@ -1084,6 +1084,47 @@ static int ide_dvd_read_structure(IDEState *s, int format, } } +static unsigned int event_status_media (IDEState *s, +uint8_t *buf, +unsigned int max_len, +unsigned int event_class, +unsigned int supported_events) +{ +enum media_event_code { + no_change =0 ,/* Status unchanged */ + eject_requested,/* received a request from user to eject */ + new_media,/* new media inserted and ready for access */ + media_removal,/* only for media changers */ + media_changed,/* only for media changers */ + bg_format_completed,/* MRW or DVD+RW b/g format completed */ + bg_format_restarted,/* MRW or DVD+RW b/g format restarted */ + }; +enum media_status { + tray_open =1 , + media_present =2 , + }; +uint8_t event_code, media_status; + + media_status =0 ; +if (s->bs->tray_open) { + media_status = tray_open; + }else if (bdrv_is_inserted (s->bs)) { + media_status = media_present; + } + +/* Event notification descriptor */
hw/ide/core.c static int ide_dvd_read_structure(IDEState *s, int format, · line 1125
Quoted 10 lines
hw/ide/core.c +10
-0+ event_code = no_change; +if (media_status != tray_open && s->events.new_media) { + event_code = new_media; + s->events.new_media = false; + } + + buf[4 ] = event_code; + buf[5 ] = media_status; + +return 6 ;/* We wrote to just 2 extra bytes from the header */
I must admit that I don't understand your answer you gave on v1 here. Let me quote:
Quoted 4 lines
Quoted 2 lines
Quoted 2 lines
After
media_state, there are two more fields for start/end slot (even though they are reserved because we don't have a multiple slot device)Yes, they're reserved, so we shouldn't change them. Any change might trigger bad response from guests.
I'm not sure what we would be _changing_. We're building a response structure here, not modifying existing data.
So what your code does is to receive a short response that leaves out the reserved fields. I don't think this is how it's supposed to work. We should include the reserved fields in the descriptor length and zero them (MMC-5, section 3.6.8):
"“Reserved” is a keyword referring to bits, bytes, words, fields and code values that are set aside for future standardization. A reserved bit, byte, word or field shall be set to zero, or in accordance with a future extension to this standard."
Quoted 43 lines
hw/ide/core.c +26
-2+} +static unsigned int event_status_nea (uint8_t *buf,unsigned int max_len) {cpu_to_ube16 (buf,0x00 );/* No event descriptor returned */ @@ -1107,7 +1148,28 @@ static void handle_get_event_status_notification(IDEState *s,uint8_t len_lsb;uint8_t control; } __attribute__((packed)) *gesn_cdb; +enum notification_class_request_type { + reserved1 =1 <<0 , + operational_change =1 <<1 , + power_management =1 <<2 , + external_request =1 <<3 , + media =1 <<4 , + multi_host =1 <<5 , + device_busy =1 <<6 , + reserved2 =1 <<7 , + }; +enum event_notification_class_field { + enc_no_events =0 , + enc_operational_change, + enc_power_management, + enc_external_request, + enc_media, + enc_multiple_hosts, + enc_device_busy, + enc_reserved, + };unsigned int max_len, used_len; +unsigned int supported_events; gesn_cdb = (void *)packet; max_len =ube16_to_cpu (&gesn_cdb->len_msb);@@ -1122,8 +1184,33 @@ static void handle_get_event_status_notification(IDEState *s,/* polling mode operation */ -/* We don't support any event class (yet). */ - used_len =event_status_nea (buf, max_len); +/* +* These are the supported events. +*
hw/ide/core.c static void handle_get_event_status_notification(IDEState *s, · line 1199
Quoted 10 lines
hw/ide/core.c +10
-0+* We currently only support requests of the 'media' type. +*/ + supported_events = media; + +/* +* Event notification header; will be overwritten by the +* NO_EVENT_AVAILABLE code if we don't have events: according to +* MMC-5 6.7.2.2, if nea = 1, event class field should be 0. +*/ +cpu_to_ube16 (buf, max_len);
That doesn't look right. I think it needs to be used_len - sizeof(header).
hw/ide/core.c static void handle_get_event_status_notification(IDEState *s, · line 1201
Quoted 2 lines
hw/ide/core.c +2
-0+ buf[2 ] = event_class; + buf[3 ] = supported_events;
The spec doesn't talk about "event class" for buf[2], but about "notification class" (same applies for the comment above). We should stick to that as buf[3] is called "supported event class", so they might be confused.
hw/ide/core.c static void handle_get_event_status_notification(IDEState *s, · line 1212
Quoted 11 lines
hw/ide/core.c +11
-0+ +/* +* Responses to requests are to be based on request priority. The +* notification_class_request_type enum above specifies the +* priority: upper elements are higher prio than lower ones. +*/ +if (gesn_cdb->request & media) { + used_len =event_status_media (s, buf, max_len, enc_media, + supported_events); + }else { + used_len =event_status_nea (buf, max_len);
This expands to:
cpu_to_ube16(buf, 0x00); /* No event descriptor returned */
buf[2] = 0x80; /* No Event Available (NEA) */
buf[3] = 0x00; /* Empty supported event classes */The first one is covered if you fix what I mentioned above (used_len - sizeof(header)). The third one is wrong, we still support the media class. So what is left is one line:
buf[2] = 0x80; /* No Event Available (NEA) */
I'm still not convinced that having event_status_nea() as a separate function makes a lot of sense.
Quoted 29 lines
hw/ide/core.c +2
-0+ }ide_atapi_cmd_reply (s, used_len, max_len); }@@ -1650,6 +1737,7 @@ static void cdrom_change_cb(void *opaque, int reason) s->sense_key =SENSE_UNIT_ATTENTION ; s->asc =ASC_MEDIUM_MAY_HAVE_CHANGED ; s->cdrom_changed =1 ; + s->events.new_media = true;ide_set_irq (s->bus); }hw/ide/internal.h +5
-0@@ -373,6 +373,11 @@ typedef int DMAFunc(IDEDMA *);typedef int DMAIntFunc (IDEDMA *,int );typedef void DMARestartFunc (void *,int ,int ); +struct unreported_events { +bool eject_request; +bool new_media; +}; +/* NOTE: IDEState represents in fact one drive */ struct IDEState {IDEBus *bus;
hw/ide/internal.h struct IDEState { · line 419
Quoted 7 lines
hw/ide/internal.h +1
-0@@ -408,6 +413,7 @@ struct IDEState {BlockDriverState *bs;char version[9 ];/* ATAPI specific */ +struct unreported_events events;uint8_t sense_key;uint8_t asc;uint8_t cdrom_changed;
Kevin
PATCH v2 6/6 atapi: Save / load the new GESN event states during migration
In reply to Amit ShahAdd a new subsection to save and restore the events state for the GET_EVENT_STATUS_NOTIFICATION command.
--- hw/ide/core.c | 22 ++++++++++++++++++++++ 1 files changed, 22 insertions(+), 0 deletions(-)
hw/ide/core.c +22 -0
@@ -2882,6 +2882,25 @@ static bool ide_drive_pio_state_needed(void *opaque)return (s->status &DRQ_STAT ) !=0 ; } +static bool ide_atapi_gesn_needed (void *opaque) +{ +IDEState *s = opaque; + +return s->events.new_media || s->events.eject_request; +} + +/* Fields for GET_EVENT_STATUS_NOTIFICATION ATAPI command */ +const VMStateDescription vmstate_ide_atapi_gesn_state = { + .name ="ide_drive/atapi/gesn_state" , + .version_id =1 , + .minimum_version_id =1 , + .minimum_version_id_old =1 , + .fields = (VMStateField []) { +VMSTATE_BOOL (events.new_media, IDEState), +VMSTATE_BOOL (events.eject_request, IDEState), + } +}; +const VMStateDescription vmstate_ide_drive_pio_state = { .name ="ide_drive/pio_state" , .version_id =1 ,
@@ -2936,6 +2955,9 @@ const VMStateDescription vmstate_ide_drive = { .vmsd = &vmstate_ide_drive_pio_state, .needed = ide_drive_pio_state_needed, }, { + .vmsd = &vmstate_ide_atapi_gesn_state, + .needed = ide_atapi_gesn_needed, + }, {/* empty */ } }
Signature · 1 line
1.7.4.2
Re: [Qemu-devel] [PATCH v2 6/6] atapi: Save / load the new GESN event states during migration
Am 12.04.2011 11:27, schrieb Amit Shah:
Quoted 4 lines
Add a new subsection to save and restore the events state for the
GET_EVENT_STATUS_NOTIFICATIONcommand.Signed-off-by Amit Shah <[email protected]>
Looks good, but I think it should be merged into patch 5, so that there are no commits for which migration is broken.
Kevin