qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] vl: disable default cdrom when using explicitely scsi-hd
@ 2016-12-31 14:34 Hervé Poussineau
  2017-01-09 12:49 ` Markus Armbruster
  0 siblings, 1 reply; 6+ messages in thread
From: Hervé Poussineau @ 2016-12-31 14:34 UTC (permalink / raw)
  To: qemu-devel
  Cc: qemu-trivial, Michael Tokarev, Laurent Vivier,
	Hervé Poussineau, Paolo Bonzini

'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(+)

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       },
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] vl: disable default cdrom when using explicitely scsi-hd
  2016-12-31 14:34 [Qemu-devel] [PATCH] vl: disable default cdrom when using explicitely scsi-hd Hervé Poussineau
@ 2017-01-09 12:49 ` Markus Armbruster
  2017-01-09 13:48   ` Paolo Bonzini
  0 siblings, 1 reply; 6+ messages in thread
From: Markus Armbruster @ 2017-01-09 12:49 UTC (permalink / raw)
  To: Hervé Poussineau
  Cc: qemu-devel, qemu-trivial, Michael Tokarev, Laurent Vivier,
	Paolo Bonzini

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       },

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] vl: disable default cdrom when using explicitely scsi-hd
  2017-01-09 12:49 ` Markus Armbruster
@ 2017-01-09 13:48   ` Paolo Bonzini
  2017-02-18 17:58     ` Hervé Poussineau
  0 siblings, 1 reply; 6+ messages in thread
From: Paolo Bonzini @ 2017-01-09 13:48 UTC (permalink / raw)
  To: Markus Armbruster, Hervé Poussineau
  Cc: qemu-trivial, Michael Tokarev, qemu-devel, Laurent Vivier



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       },
> 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] vl: disable default cdrom when using explicitely scsi-hd
  2017-01-09 13:48   ` Paolo Bonzini
@ 2017-02-18 17:58     ` Hervé Poussineau
  2017-02-20  1:00       ` Markus Armbruster
  0 siblings, 1 reply; 6+ messages in thread
From: Hervé Poussineau @ 2017-02-18 17:58 UTC (permalink / raw)
  To: Paolo Bonzini, Markus Armbruster
  Cc: qemu-trivial, Michael Tokarev, qemu-devel, Laurent Vivier

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       },
>>
>>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] vl: disable default cdrom when using explicitely scsi-hd
  2017-02-18 17:58     ` Hervé Poussineau
@ 2017-02-20  1:00       ` Markus Armbruster
  2017-02-20 19:54         ` John Snow
  0 siblings, 1 reply; 6+ messages in thread
From: Markus Armbruster @ 2017-02-20  1:00 UTC (permalink / raw)
  To: Hervé Poussineau
  Cc: Paolo Bonzini, qemu-trivial, Michael Tokarev, qemu-devel,
	Laurent Vivier, John Snow, qemu-block

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] [PATCH] vl: disable default cdrom when using explicitely scsi-hd
  2017-02-20  1:00       ` Markus Armbruster
@ 2017-02-20 19:54         ` John Snow
  0 siblings, 0 replies; 6+ messages in thread
From: John Snow @ 2017-02-20 19:54 UTC (permalink / raw)
  To: Markus Armbruster, Hervé Poussineau
  Cc: Paolo Bonzini, qemu-trivial, Michael Tokarev, qemu-devel,
	Laurent Vivier, qemu-block



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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-02-20 19:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-31 14:34 [Qemu-devel] [PATCH] vl: disable default cdrom when using explicitely scsi-hd Hervé Poussineau
2017-01-09 12:49 ` Markus Armbruster
2017-01-09 13:48   ` Paolo Bonzini
2017-02-18 17:58     ` Hervé Poussineau
2017-02-20  1:00       ` Markus Armbruster
2017-02-20 19:54         ` John Snow

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).