• Michael Roth's avatar
    qemu-char: don't issue CHR_EVENT_OPEN in a BH · bd5c51ee
    Michael Roth authored
    When CHR_EVENT_OPENED was initially added, it was CHR_EVENT_RESET,
    and it was issued as a bottom-half:
    
    86e94dea
    
    Which we basically used to print out a greeting/prompt for the
    monitor.
    
    AFAICT the only reason this was ever done in a BH was because in
    some cases we'd modify the chr_write handler for a new chardev
    backend *after* the site where we issued the reset (see:
    86e94d:qemu_chr_open_stdio())
    
    At some point this event was renamed to CHR_EVENT_OPENED, and we've
    maintained the use of this BH ever since.
    
    However, due to 9f939df9, we schedule
    the BH via g_idle_add(), which is causing events to sometimes be
    delivered after we've already begun processing data from backends,
    leading to:
    
     known bugs:
    
      QMP:
        session negotation resets with OPENED event, in some cases this
        is causing new sessions to get sporadically reset
    
     potential bugs:
    
      hw/usb/redirect.c:
        can_read handler checks for dev->parser != NULL, which may be
        true if CLOSED BH has not been executed yet. In the past, OPENED
        quiesced outstanding CLOSED events prior to us reading client
        data. If it's delayed, our check may allow reads to occur even
        though we haven't processed the OPENED event yet, and when we
        do finally get the OPENED event, our state may get reset.
    
      qtest.c:
        can begin session before OPENED event is processed, leading to
        a spurious reset of the system and irq_levels
    
      gdbstub.c:
        may start a gdb session prior to the machine being paused
    
    To fix these, let's just drop the BH.
    
    Since the initial reasoning for using it still applies to an extent,
    work around that by deferring the delivery of CHR_EVENT_OPENED until
    after the chardevs have been fully initialized, toward the end of
    qmp_chardev_add() (or some cases, qemu_chr_new_from_opts()). This
    defers delivery long enough that we can be assured a CharDriverState
    is fully initialized before CHR_EVENT_OPENED is sent.
    
    Also, rather than requiring each chardev to do an explicit open, do it
    automatically, and allow the small few who don't desire such behavior to
    suppress the OPENED-on-init behavior by setting a 'explicit_be_open'
    flag.
    
    We additionally add missing OPENED events for stdio backends on w32,
    which were previously not being issued, causing us to not recieve the
    banner and initial prompts for qmp/hmp.
    Reported-by: default avatarStefan Priebe <s.priebe@profihost.ag>
    Signed-off-by: default avatarMichael Roth <mdroth@linux.vnet.ibm.com>
    Message-id: 1370636393-21044-1-git-send-email-mdroth@linux.vnet.ibm.com
    Cc: qemu-stable@nongnu.org
    Signed-off-by: default avatarMichael Roth <mdroth@linux.vnet.ibm.com>
    Signed-off-by: default avatarAnthony Liguori <aliguori@us.ibm.com>
    bd5c51ee
baum.c 19.7 KB