Skip to content
  • Eric Blake's avatar
    5229564b
    test-qga: Actually test 0xff sync bytes · 5229564b
    Eric Blake authored
    
    
    Commit 62c39b30 introduced test-qga, and at face value, appears
    to be testing the 'guest-sync' behavior that is recommended for
    guests in sending 0xff to QGA to force the parser to reset.  But
    this aspect of the test has never actually done anything: the
    qmp_fd() call chain converts its string argument into QObject,
    then converts that QObject back to the actual string that is
    sent over the wire - and the conversion process silently drops
    the 0xff byte from the string sent to QGA, thus never resetting
    the QGA parser.
    
    An upcoming patch will get rid of the wasteful round trip
    through QObject, at which point the string in test-qga will be
    directly sent over the wire.
    
    But fixing qmp_fd() to actually send 0xff over the wire is not
    all we have to do - the actual QMP parser loudly complains that
    0xff is not valid JSON, and sends an error message _prior_ to
    actually parsing the 'guest-sync' or 'guest-sync-delimited'
    command.  With 'guest-sync', we cannot easily tell if this error
    message is a result of our command - which is WHY we invented
    the 'guest-sync-delimited' command.  So for the testsuite, fix
    things to only check 0xff behavior on 'guest-sync-delimited',
    and to loop until we've consumed all garbage prior to the
    requested delimiter, which is compatible with the documented actions
    that a real QGA client is supposed to do.
    
    Ideally, we'd fix the QGA JSON parser to silently ignore 0xff
    rather than sending an error message back, at which point we
    could enhance this test for 'guest-sync' as well as for
    'guest-sync-delimited'.  But for the sake of this patch, our
    testing of 'guest-sync' is no worse than it was pre-patch,
    because we have never been sending 0xff over the wire in the
    first place.
    
    Signed-off-by: default avatarEric Blake <eblake@redhat.com>
    Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
    Message-Id: <20170427215821.19397-11-eblake@redhat.com>
    Reviewed-by: default avatarMichael Roth <mdroth@linux.vnet.ibm.com>
    [Additional comment squashed in, along with matching commit message
    update]
    Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
    5229564b
    test-qga: Actually test 0xff sync bytes
    Eric Blake authored
    
    
    Commit 62c39b30 introduced test-qga, and at face value, appears
    to be testing the 'guest-sync' behavior that is recommended for
    guests in sending 0xff to QGA to force the parser to reset.  But
    this aspect of the test has never actually done anything: the
    qmp_fd() call chain converts its string argument into QObject,
    then converts that QObject back to the actual string that is
    sent over the wire - and the conversion process silently drops
    the 0xff byte from the string sent to QGA, thus never resetting
    the QGA parser.
    
    An upcoming patch will get rid of the wasteful round trip
    through QObject, at which point the string in test-qga will be
    directly sent over the wire.
    
    But fixing qmp_fd() to actually send 0xff over the wire is not
    all we have to do - the actual QMP parser loudly complains that
    0xff is not valid JSON, and sends an error message _prior_ to
    actually parsing the 'guest-sync' or 'guest-sync-delimited'
    command.  With 'guest-sync', we cannot easily tell if this error
    message is a result of our command - which is WHY we invented
    the 'guest-sync-delimited' command.  So for the testsuite, fix
    things to only check 0xff behavior on 'guest-sync-delimited',
    and to loop until we've consumed all garbage prior to the
    requested delimiter, which is compatible with the documented actions
    that a real QGA client is supposed to do.
    
    Ideally, we'd fix the QGA JSON parser to silently ignore 0xff
    rather than sending an error message back, at which point we
    could enhance this test for 'guest-sync' as well as for
    'guest-sync-delimited'.  But for the sake of this patch, our
    testing of 'guest-sync' is no worse than it was pre-patch,
    because we have never been sending 0xff over the wire in the
    first place.
    
    Signed-off-by: default avatarEric Blake <eblake@redhat.com>
    Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
    Message-Id: <20170427215821.19397-11-eblake@redhat.com>
    Reviewed-by: default avatarMichael Roth <mdroth@linux.vnet.ibm.com>
    [Additional comment squashed in, along with matching commit message
    update]
    Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
Loading