Re: [Qemu-stable] [Qemu-devel] [PATCH] mux: fix ctrl-a b again

[Philippe Mathieu-Daudé]

On 04/18/2018 11:32 AM, Philippe Mathieu-Daudé wrote:
> Hi Marc-André,
> 
> On 04/18/2018 07:36 AM, Marc-André Lureau wrote:
>> In  commit cd9526ab7c04f2c32c63340b04401f6ed25682b9
>> Author: Philippe Mathieu-Daudé <address@hidden>
>> Date:   Thu Mar 8 23:39:32 2018 +0100
>>
>>     hw/isa/superio: Factor out the serial code from pc87312.c
>>
>> You changed from:
>>
>> -            if (chr == NULL) {
>> -                snprintf(name, sizeof(name), "ser%d", i);
>> -                chr = qemu_chr_new(name, "null");
>> -            }
>>
>> to:
>>
>> +            if (chr == NULL || chr->be) {
>> +                name = g_strdup_printf("discarding-serial%d", i);
>> +                chr = qemu_chr_new(name, "null");
>> +            } else {
>> +                name = g_strdup_printf("serial%d", i);
>> +            }
>>
>> Why do you check if chr->be ? In case of a mux chardev, it may already
>> have an active frontend (yeah be is CharBackend which is the frontend,
>> I still can't grasp that either, please Paolo change your mind! ;).
>> And this is the job for a mux chardev, handling several frontends.
> 
> I was afraid this refactor could be related.
> 
> IIRC I had troubles running "qemu-system-alpha -append console=srm" but
> I tried again with/without the chr->be check and it works fine...
> 
> Anyway the new code is buggy, this is wrong (simplified):
> 
>  if (chr->be) chr = qemu_chr_new(name, "null");

Indeed with 1b2503fcf7b59 applied and removing the 'chr->be' check I can
successfully boot my mips/alpha images.

-- >8 --
diff --git a/hw/isa/isa-superio.c b/hw/isa/isa-superio.c
@@ -43,7 +43,7 @@ static void isa_superio_realize(DeviceState *dev,
Error **errp)
         if (!k->parallel.is_enabled || k->parallel.is_enabled(sio, i)) {
             /* FIXME use a qdev chardev prop instead of parallel_hds[] */
             chr = parallel_hds[i];
-            if (chr == NULL || chr->be) {
+            if (chr == NULL) {
                 name = g_strdup_printf("discarding-parallel%d", i);
                 chr = qemu_chr_new(name, "null");
             } else {
@@ -83,7 +83,7 @@ static void isa_superio_realize(DeviceState *dev,
Error **errp)
         if (!k->serial.is_enabled || k->serial.is_enabled(sio, i)) {
             /* FIXME use a qdev chardev prop instead of serial_hds[] */
             chr = serial_hds[i];
-            if (chr == NULL || chr->be) {
+            if (chr == NULL) {
                 name = g_strdup_printf("discarding-serial%d", i);
                 chr = qemu_chr_new(name, "null");
             } else {
--
2.17.0