Re: [Qemu-devel] [PATCH] Add info commands for serial/parallel devices

[Anthony Liguori <anthony@codemonkey.ws>]

Thiemo Seufer wrote:
> Anthony Liguori wrote:
>   
>> Howdy,
>>
>> The following patch adds an info serial and an info parallel command.  
>> Besides providing useful information (especially for the serial port), 
>> it provides a method for management tools to connect to a running VM and 
>> what character devices the serial/parallel ports have been redirected to.
>>
>> The format of the info is similar to that of info block.
>>     
> [snip]
>   
>> diff -r 18e99d1e8814 vl.c
>> --- a/vl.c	Sat Mar 03 21:18:48 2007 -0600
>> +++ b/vl.c	Sat Mar 03 21:33:07 2007 -0600
>> @@ -2884,66 +2884,73 @@ CharDriverState *qemu_chr_open(const cha
>>  CharDriverState *qemu_chr_open(const char *filename)
>>  {
>>      const char *p;
>> +    CharDriverState *chr;
>>  
>>      if (!strcmp(filename, "vc")) {
>> -        return text_console_init(&display_state);
>> +        chr = text_console_init(&display_state);
>>      } else if (!strcmp(filename, "null")) {
>> -        return qemu_chr_open_null();
>> +        chr = qemu_chr_open_null();
>>      } else 
>>      if (strstart(filename, "tcp:", &p)) {
>> -        return qemu_chr_open_tcp(p, 0, 0);
>> +        chr = qemu_chr_open_tcp(p, 0, 0);
>>      } else
>>      if (strstart(filename, "telnet:", &p)) {
>> -        return qemu_chr_open_tcp(p, 1, 0);
>> +        chr = qemu_chr_open_tcp(p, 1, 0);
>>      } else
>>      if (strstart(filename, "udp:", &p)) {
>> -        return qemu_chr_open_udp(p);
>> +        chr = qemu_chr_open_udp(p);
>>      } else
>>      if (strstart(filename, "mon:", &p)) {
>>          CharDriverState *drv = qemu_chr_open(p);
>>          if (drv) {
>>              drv = qemu_chr_open_mux(drv);
>>              monitor_init(drv, !nographic);
>> -            return drv;
>> -        }
>> -        printf("Unable to open driver: %s\n", p);
>> -        return 0;
>> +            chr = drv;
>> +        } else {
>> +	    printf("Unable to open driver: %s\n", p);
>> +	    return 0;
>> +	}
>>      } else
>>  #ifndef _WIN32
>>      if (strstart(filename, "unix:", &p)) {
>> -	return qemu_chr_open_tcp(p, 0, 1);
>> +	chr = qemu_chr_open_tcp(p, 0, 1);
>>      } else if (strstart(filename, "file:", &p)) {
>> -        return qemu_chr_open_file_out(p);
>> +        chr = qemu_chr_open_file_out(p);
>>      } else if (strstart(filename, "pipe:", &p)) {
>> -        return qemu_chr_open_pipe(p);
>> +        chr = qemu_chr_open_pipe(p);
>>      } else if (!strcmp(filename, "pty")) {
>> -        return qemu_chr_open_pty();
>> +        chr = qemu_chr_open_pty();
>>      } else if (!strcmp(filename, "stdio")) {
>> -        return qemu_chr_open_stdio();
>> +        chr = qemu_chr_open_stdio();
>>      } else 
>>  #endif
>>  #if defined(__linux__)
>>      if (strstart(filename, "/dev/parport", NULL)) {
>> -        return qemu_chr_open_pp(filename);
>> +        chr = qemu_chr_open_pp(filename);
>>      } else 
>>      if (strstart(filename, "/dev/", NULL)) {
>> -        return qemu_chr_open_tty(filename);
>> +        chr = qemu_chr_open_tty(filename);
>>      } else 
>>  #endif
>>  #ifdef _WIN32
>>      if (strstart(filename, "COM", NULL)) {
>> -        return qemu_chr_open_win(filename);
>> +        chr = qemu_chr_open_win(filename);
>>      } else
>>      if (strstart(filename, "pipe:", &p)) {
>> -        return qemu_chr_open_win_pipe(p);
>> +        chr = qemu_chr_open_win_pipe(p);
>>      } else
>>      if (strstart(filename, "file:", &p)) {
>> -        return qemu_chr_open_win_file_out(p);
>> -    }
>> +        chr = qemu_chr_open_win_file_out(p);
>> +    } else
>>  #endif
>>      {
>>          return NULL;
>>      }
>> +
>> +    if (chr)
>> +	chr->filename = strdup(filename);
>> +
>> +    return chr;
>>     
>
> Why is this part needed?
>   

So that info seral|parallel can spit out the name used to create open 
the device.

It has to be strdup()'d b/c it's passed in as a const char.  There's no 
guarantee that memory will stay around.

>>  }
>>  
>>  void qemu_chr_close(CharDriverState *chr)
>> diff -r 18e99d1e8814 vl.h
>> --- a/vl.h	Sat Mar 03 21:18:48 2007 -0600
>> +++ b/vl.h	Sat Mar 03 21:33:07 2007 -0600
>> @@ -307,6 +307,7 @@ typedef struct CharDriverState {
>>      void *opaque;
>>      int focus;
>>      QEMUBH *bh;
>> +    char *filename;
>>  } CharDriverState;
>>     
>
> const char * ?
>   

To me, const char * always implies that you don't own the memory.  This 
is helped by the fact that free doesn't take a const void * and newer 
GCC's will complain if you free() a const char *.

Regards,

Anthony Liguori

> Thiemo
>
>