Re: [PATCH v2 10/18] tests/qtest/migration: Add a test for HMP completion

["Dr. David Alan Gilbert" <dave@treblig.org>]

* Fabiano Rosas (farosas@suse.de) wrote:
> "Dr. David Alan Gilbert" <dave@treblig.org> writes:
> 
> > * Fabiano Rosas (farosas@suse.de) wrote:
> >> The following patches will touch HMP readline completion for migration
> >> parameters, add a test case.
> >> 
> >> The test puts the monitor on a chardev via socket and bypasses qtest
> >> facilities because it needs to emit raw codes to readline. It
> >> therefore requires a couple of new helpers to read/write to the
> >> monitor socket.
> >> 
> >> Usage:
> >> QTEST_QEMU_BINARY=./qemu-system-x86_64 \
> >> ./tests/qtest/migration-test --full -p /x86_64/migration/hmp/completion
> >> 
> >> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> >> ---
> >
> >> +static void hmp_sock_read(int fd, char *buf, size_t buf_sz)
> >> +{
> >> +    char *p = buf;
> >> +    size_t sz = buf_sz - 1;
> >> +
> >> +    assert(fd >= 0);
> >> +    memset(buf, 0, buf_sz);
> >> +
> >> +    while (sz > 0) {
> >> +        ssize_t r = read(fd, p, sz);
> >> +        char *prompt;
> >> +
> >> +        if (!r) {
> >> +            break;
> >> +        } else if (r < 0) {
> >> +            if (errno == EINTR) {
> >> +                continue;
> >> +            }
> >> +            g_assert_not_reached();
> >> +        }
> >> +
> >> +        p += r;
> >> +        sz -= r;
> >> +
> >> +        prompt = strstr(buf, "(qemu) ");
> >> +        if (prompt) {
> >> +            *prompt = '\0';
> >
> > I'm a bit confused by what you want the output to look like after this,
> > are you assuming this is the end of the string you're reading so truncation
> > is OK?
> >
> 
> Yes, it's how I'm detecting the end of input. Here's a printf("%s|\n", buf)
> before the truncation:
> 
> 1) Read banner:                                                                                    
> 
> QEMU 11.1.50 monitor - type 'help' for more information
> (qemu) |

Oh right, that makes sense - please add a comment above the function something like
'read the output until we hit the next (qemu) prompt'

Dave

> 2) Write completion input 'migra\t'. Then read back:
> 
> migra
> migrate                 migrate_cancel          migrate_continue
> migrate_incoming        migrate_pause           migrate_recover
> migrate_set_capability  migrate_set_parameter   migrate_start_postcopy
> (qemu) |
> 
> 3) Write completion input 'info migra\t'. Then read back:
> 
> info migra
> migrate               migrate_capabilities  migrate_parameters
> (qemu) |
> 
> I think this is correct? Let me know if I missed something.
> 
> >> +            break;
> >> +        }
> >> +    }
> >> +}
> >> +
> >> +static int comp(const void *a, const void *b)
> >> +{
> >> +    return strcmp(*(const char **) a, *(const char **) b);
> >> +}
> >
> > Can you use qemu_pstrcmp0 there? (Maybe with g_ptr_array and g_ptr_array_sort is
> > what adds to some of the other questions below?)
> >
> 
> Good idea, specially the g_ptr_array. I'll update the code.
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/