| Message ID | 1483194856-14079-1-git-send-email-hpoussin@reactos.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hervé Poussineau <hpoussin@reactos.org> writes: > 'ide-hd', 'ide-cd' and 'scsi-cd' devices already disable default cdrom. > Make it the same for 'scsi-hd'. > > That way, we can add/replace the device on lun=2 without using -nodefaults. Yes, but it might upset existing usage that relies on the default CD-ROM. In my opinion, making your needs explicit is better than relying on defaults, but that doesn't mean we can change the defaults unthinkingly. Definitely not qemu-trivial. Opinions on the change? > Signed-off-by: Hervé Poussineau <hpoussin@reactos.org> > --- > vl.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/vl.c b/vl.c > index d77dd86..da97fe2 100644 > --- a/vl.c > +++ b/vl.c > @@ -223,6 +223,7 @@ static struct { > { .driver = "ide-hd", .flag = &default_cdrom }, > { .driver = "ide-drive", .flag = &default_cdrom }, > { .driver = "scsi-cd", .flag = &default_cdrom }, > + { .driver = "scsi-hd", .flag = &default_cdrom }, > { .driver = "virtio-serial-pci", .flag = &default_virtcon }, > { .driver = "virtio-serial", .flag = &default_virtcon }, > { .driver = "VGA", .flag = &default_vga },
On 09/01/2017 13:49, Markus Armbruster wrote: > Hervé Poussineau <hpoussin@reactos.org> writes: > >> 'ide-hd', 'ide-cd' and 'scsi-cd' devices already disable default cdrom. >> Make it the same for 'scsi-hd'. >> >> That way, we can add/replace the device on lun=2 without using -nodefaults. > > Yes, but it might upset existing usage that relies on the default > CD-ROM. In my opinion, making your needs explicit is better than > relying on defaults, but that doesn't mean we can change the defaults > unthinkingly. Definitely not qemu-trivial. > > Opinions on the change? The original rationale for the change was "ide-hd has to suppress the default CD-ROM, or else you can't put one on secondary master without -nodefaults" but the same applies for scsi-hd vs. lun=1. So I'm not sure, but I lean towards accepting the patch. Paolo >> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org> >> --- >> vl.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/vl.c b/vl.c >> index d77dd86..da97fe2 100644 >> --- a/vl.c >> +++ b/vl.c >> @@ -223,6 +223,7 @@ static struct { >> { .driver = "ide-hd", .flag = &default_cdrom }, >> { .driver = "ide-drive", .flag = &default_cdrom }, >> { .driver = "scsi-cd", .flag = &default_cdrom }, >> + { .driver = "scsi-hd", .flag = &default_cdrom }, >> { .driver = "virtio-serial-pci", .flag = &default_virtcon }, >> { .driver = "virtio-serial", .flag = &default_virtcon }, >> { .driver = "VGA", .flag = &default_vga }, > >
Hi, Le 09/01/2017 à 14:48, Paolo Bonzini a écrit : > > > On 09/01/2017 13:49, Markus Armbruster wrote: >> Hervé Poussineau <hpoussin@reactos.org> writes: >> >>> 'ide-hd', 'ide-cd' and 'scsi-cd' devices already disable default cdrom. >>> Make it the same for 'scsi-hd'. >>> >>> That way, we can add/replace the device on lun=2 without using -nodefaults. >> >> Yes, but it might upset existing usage that relies on the default >> CD-ROM. In my opinion, making your needs explicit is better than >> relying on defaults, but that doesn't mean we can change the defaults >> unthinkingly. Definitely not qemu-trivial. >> >> Opinions on the change? > > The original rationale for the change was "ide-hd has to suppress the > default CD-ROM, or else you can't put one on secondary master without > -nodefaults" but the same applies for scsi-hd vs. lun=1. > > So I'm not sure, but I lean towards accepting the patch. > > Paolo Paolo, Markus, so what is the conclusion? Accepting the patch, or refusing it? Regards, Hervé > >>> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org> >>> --- >>> vl.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/vl.c b/vl.c >>> index d77dd86..da97fe2 100644 >>> --- a/vl.c >>> +++ b/vl.c >>> @@ -223,6 +223,7 @@ static struct { >>> { .driver = "ide-hd", .flag = &default_cdrom }, >>> { .driver = "ide-drive", .flag = &default_cdrom }, >>> { .driver = "scsi-cd", .flag = &default_cdrom }, >>> + { .driver = "scsi-hd", .flag = &default_cdrom }, >>> { .driver = "virtio-serial-pci", .flag = &default_virtcon }, >>> { .driver = "virtio-serial", .flag = &default_virtcon }, >>> { .driver = "VGA", .flag = &default_vga }, >> >> >
Hervé Poussineau <hpoussin@reactos.org> writes: > Hi, > > Le 09/01/2017 à 14:48, Paolo Bonzini a écrit : >> >> >> On 09/01/2017 13:49, Markus Armbruster wrote: >>> Hervé Poussineau <hpoussin@reactos.org> writes: >>> >>>> 'ide-hd', 'ide-cd' and 'scsi-cd' devices already disable default cdrom. >>>> Make it the same for 'scsi-hd'. >>>> >>>> That way, we can add/replace the device on lun=2 without using -nodefaults. >>> >>> Yes, but it might upset existing usage that relies on the default >>> CD-ROM. In my opinion, making your needs explicit is better than >>> relying on defaults, but that doesn't mean we can change the defaults >>> unthinkingly. Definitely not qemu-trivial. >>> >>> Opinions on the change? >> >> The original rationale for the change was "ide-hd has to suppress the >> default CD-ROM, or else you can't put one on secondary master without >> -nodefaults" but the same applies for scsi-hd vs. lun=1. >> >> So I'm not sure, but I lean towards accepting the patch. >> >> Paolo > > Paolo, Markus, so what is the conclusion? > Accepting the patch, or refusing it? Suggest to repost with the commit message updated to mention the backwards incompatibility, and why you think it's okay. cc: John Snow <jsnow@redhat.com>, cc: qemu-block@nongnu.org
On 02/19/2017 08:00 PM, Markus Armbruster wrote: > Hervé Poussineau <hpoussin@reactos.org> writes: > >> Hi, >> >> Le 09/01/2017 à 14:48, Paolo Bonzini a écrit : >>> >>> >>> On 09/01/2017 13:49, Markus Armbruster wrote: >>>> Hervé Poussineau <hpoussin@reactos.org> writes: >>>> >>>>> 'ide-hd', 'ide-cd' and 'scsi-cd' devices already disable default cdrom. >>>>> Make it the same for 'scsi-hd'. >>>>> >>>>> That way, we can add/replace the device on lun=2 without using -nodefaults. >>>> >>>> Yes, but it might upset existing usage that relies on the default >>>> CD-ROM. In my opinion, making your needs explicit is better than >>>> relying on defaults, but that doesn't mean we can change the defaults >>>> unthinkingly. Definitely not qemu-trivial. >>>> >>>> Opinions on the change? >>> >>> The original rationale for the change was "ide-hd has to suppress the >>> default CD-ROM, or else you can't put one on secondary master without >>> -nodefaults" but the same applies for scsi-hd vs. lun=1. >>> >>> So I'm not sure, but I lean towards accepting the patch. >>> >>> Paolo >> >> Paolo, Markus, so what is the conclusion? >> Accepting the patch, or refusing it? > > Suggest to repost with the commit message updated to mention the > backwards incompatibility, and why you think it's okay. > cc: John Snow <jsnow@redhat.com>, cc: qemu-block@nongnu.org > I don't have a lot of history with the SCSI devices, so I'd be pretty much relying exclusively on a statement on what breaks with the change, and why that breakage would be justified. No strong feelings for/against right now and am likely to just defer to Paolo, who was leaning towards accepting it. --js
diff --git a/vl.c b/vl.c index d77dd86..da97fe2 100644 --- a/vl.c +++ b/vl.c @@ -223,6 +223,7 @@ static struct { { .driver = "ide-hd", .flag = &default_cdrom }, { .driver = "ide-drive", .flag = &default_cdrom }, { .driver = "scsi-cd", .flag = &default_cdrom }, + { .driver = "scsi-hd", .flag = &default_cdrom }, { .driver = "virtio-serial-pci", .flag = &default_virtcon }, { .driver = "virtio-serial", .flag = &default_virtcon }, { .driver = "VGA", .flag = &default_vga },
'ide-hd', 'ide-cd' and 'scsi-cd' devices already disable default cdrom. Make it the same for 'scsi-hd'. That way, we can add/replace the device on lun=2 without using -nodefaults. Signed-off-by: Hervé Poussineau <hpoussin@reactos.org> --- vl.c | 1 + 1 file changed, 1 insertion(+)