| Message ID | 1341222087-24920-3-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Jul 2, 2012 at 9:41 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > hw/scsi-disk.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c > index 747e756..e87e57c 100644 > --- a/hw/scsi-disk.c > +++ b/hw/scsi-disk.c > @@ -67,6 +67,7 @@ struct SCSIDiskState > bool media_changed; > bool media_event; > bool eject_request; > + uint64_t wwn; > QEMUBH *bh; > char *version; > char *serial; > @@ -587,9 +588,17 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf) > outbuf[buflen++] = 0; // not officially assigned > outbuf[buflen++] = 0; // reserved > outbuf[buflen++] = id_len; // length of data following > - > memcpy(outbuf+buflen, str, id_len); > buflen += id_len; > + > + if (s->wwn) { > + outbuf[buflen++] = 0x1; // Binary > + outbuf[buflen++] = 0x3; // NAA > + outbuf[buflen++] = 0; // reserved C99 comments. > + outbuf[buflen++] = 8; > + stq_be_p(&outbuf[buflen], s->wwn); > + buflen += 8; > + } > break; > } > case 0xb0: /* block limits */ > @@ -1924,6 +1933,7 @@ static Property scsi_hd_properties[] = { > SCSI_DISK_F_REMOVABLE, false), > DEFINE_PROP_BIT("dpofua", SCSIDiskState, features, > SCSI_DISK_F_DPOFUA, false), > + DEFINE_PROP_HEX64("wwn", SCSIDiskState, wwn, 0), > DEFINE_PROP_END_OF_LIST(), > }; > > @@ -1968,6 +1978,7 @@ static TypeInfo scsi_hd_info = { > > static Property scsi_cd_properties[] = { > DEFINE_SCSI_DISK_PROPERTIES(), > + DEFINE_PROP_HEX64("wwn", SCSIDiskState, wwn, 0), > DEFINE_PROP_END_OF_LIST(), > }; > > @@ -2029,6 +2040,7 @@ static Property scsi_disk_properties[] = { > SCSI_DISK_F_REMOVABLE, false), > DEFINE_PROP_BIT("dpofua", SCSIDiskState, features, > SCSI_DISK_F_DPOFUA, false), > + DEFINE_PROP_HEX64("wwn", SCSIDiskState, wwn, 0), > DEFINE_PROP_END_OF_LIST(), > }; > > -- > 1.7.10.2 > > >
Il 03/07/2012 21:09, Blue Swirl ha scritto: >> > @@ -587,9 +588,17 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf) >> > outbuf[buflen++] = 0; // not officially assigned >> > outbuf[buflen++] = 0; // reserved >> > outbuf[buflen++] = id_len; // length of data following >> > - >> > memcpy(outbuf+buflen, str, id_len); >> > buflen += id_len; >> > + >> > + if (s->wwn) { >> > + outbuf[buflen++] = 0x1; // Binary >> > + outbuf[buflen++] = 0x3; // NAA >> > + outbuf[buflen++] = 0; // reserved > C99 comments. > Just following the style of this code. Feel free to send a patch to replace with #defines. Paolo
On Wed, Jul 4, 2012 at 7:33 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 03/07/2012 21:09, Blue Swirl ha scritto: >>> > @@ -587,9 +588,17 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf) >>> > outbuf[buflen++] = 0; // not officially assigned >>> > outbuf[buflen++] = 0; // reserved >>> > outbuf[buflen++] = id_len; // length of data following >>> > - >>> > memcpy(outbuf+buflen, str, id_len); >>> > buflen += id_len; >>> > + >>> > + if (s->wwn) { >>> > + outbuf[buflen++] = 0x1; // Binary >>> > + outbuf[buflen++] = 0x3; // NAA >>> > + outbuf[buflen++] = 0; // reserved >> C99 comments. >> > > Just following the style of this code. Feel free to send a patch to > replace with #defines. That's not how we should work. New code should be compliant with our goals. Pushing the responsibility for fixing issues to other people does not scale. > > Paolo >
Il 05/07/2012 20:03, Blue Swirl ha scritto: > > > > + if (s->wwn) { > > > > + outbuf[buflen++] = 0x1; // Binary > > > > + outbuf[buflen++] = 0x3; // NAA > > > > + outbuf[buflen++] = 0; // reserved > > > > > > C99 comments. > > > > Just following the style of this code. Feel free to send a patch to > > replace with #defines. > > That's not how we should work. New code should be compliant with our > goals. Pushing the responsibility for fixing issues to other people > does not scale. I believe the coding style are attacking the wrong problem. It's end-of-line comments that should be avoided in favor of #defines, packed structs, designated initializers, etc. But for end-of-line comments, C++ comments are superior to /* */ comments. I do plan to fix the clarity issue with SCSI data structures and constants. But for now, the best compromise is to keep C++ comments IMHO. I'm not pushing the responsibility to other people in general, but if they think C++ comments are a major issue they can send patches. Paolo
On Fri, Jul 6, 2012 at 7:05 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > Il 05/07/2012 20:03, Blue Swirl ha scritto: >> > > > + if (s->wwn) { >> > > > + outbuf[buflen++] = 0x1; // Binary >> > > > + outbuf[buflen++] = 0x3; // NAA >> > > > + outbuf[buflen++] = 0; // reserved >> > > >> > > C99 comments. >> > >> > Just following the style of this code. Feel free to send a patch to >> > replace with #defines. >> >> That's not how we should work. New code should be compliant with our >> goals. Pushing the responsibility for fixing issues to other people >> does not scale. > > I believe the coding style are attacking the wrong problem. It's > end-of-line comments that should be avoided in favor of #defines, packed > structs, designated initializers, etc. This applies to C89 comments too. Perhaps HACKING file should mention this. > But for end-of-line comments, > C++ comments are superior to /* */ comments. Well, submit a change to CODING_STYLE then. I think the only places where C99 is better are lines like //#define DEBUG_XYZ > > I do plan to fix the clarity issue with SCSI data structures and > constants. But for now, the best compromise is to keep C++ comments > IMHO. I'm not pushing the responsibility to other people in general, > but if they think C++ comments are a major issue they can send patches. When you add them in new code, that is a problem. New code should comply with CODING_STYLE, code with other styles should be converted as it is touched. It shouldn't be much effort from your part to change the comments in this patch to C89 style. > > Paolo
Il 07/07/2012 09:48, Blue Swirl ha scritto: >> > I do plan to fix the clarity issue with SCSI data structures and >> > constants. But for now, the best compromise is to keep C++ comments >> > IMHO. I'm not pushing the responsibility to other people in general, >> > but if they think C++ comments are a major issue they can send patches. > When you add them in new code, that is a problem. New code should > comply with CODING_STYLE, code with other styles should be converted > as it is touched. > > It shouldn't be much effort from your part to change the comments in > this patch to C89 style. Sure, but that would be the wrong fix. The right fix is to augment scsi-defs.h, and avoid the need for comments completely. Paolo
diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c index 747e756..e87e57c 100644 --- a/hw/scsi-disk.c +++ b/hw/scsi-disk.c @@ -67,6 +67,7 @@ struct SCSIDiskState bool media_changed; bool media_event; bool eject_request; + uint64_t wwn; QEMUBH *bh; char *version; char *serial; @@ -587,9 +588,17 @@ static int scsi_disk_emulate_inquiry(SCSIRequest *req, uint8_t *outbuf) outbuf[buflen++] = 0; // not officially assigned outbuf[buflen++] = 0; // reserved outbuf[buflen++] = id_len; // length of data following - memcpy(outbuf+buflen, str, id_len); buflen += id_len; + + if (s->wwn) { + outbuf[buflen++] = 0x1; // Binary + outbuf[buflen++] = 0x3; // NAA + outbuf[buflen++] = 0; // reserved + outbuf[buflen++] = 8; + stq_be_p(&outbuf[buflen], s->wwn); + buflen += 8; + } break; } case 0xb0: /* block limits */ @@ -1924,6 +1933,7 @@ static Property scsi_hd_properties[] = { SCSI_DISK_F_REMOVABLE, false), DEFINE_PROP_BIT("dpofua", SCSIDiskState, features, SCSI_DISK_F_DPOFUA, false), + DEFINE_PROP_HEX64("wwn", SCSIDiskState, wwn, 0), DEFINE_PROP_END_OF_LIST(), }; @@ -1968,6 +1978,7 @@ static TypeInfo scsi_hd_info = { static Property scsi_cd_properties[] = { DEFINE_SCSI_DISK_PROPERTIES(), + DEFINE_PROP_HEX64("wwn", SCSIDiskState, wwn, 0), DEFINE_PROP_END_OF_LIST(), }; @@ -2029,6 +2040,7 @@ static Property scsi_disk_properties[] = { SCSI_DISK_F_REMOVABLE, false), DEFINE_PROP_BIT("dpofua", SCSIDiskState, features, SCSI_DISK_F_DPOFUA, false), + DEFINE_PROP_HEX64("wwn", SCSIDiskState, wwn, 0), DEFINE_PROP_END_OF_LIST(), };
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- hw/scsi-disk.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)