[Libvir] [QEMU 3/3] the qemu driver & daemon

[Karel Zak]

On Mon, Aug 28, 2006 at 12:18:52AM +0100, Daniel P. Berrange wrote:
> Attached is the patch for the qemu driver & daemon process

 Cool pathes!


 On first look (I hope I'm not too pedantic:-):


> +static int qemudParseUUID(const char *uuid,
> +			  unsigned char *rawuuid) {

 We have virParseUUID() in xml.c, I think we should maintain more than
 one version of same code.

> +  if (!(*argv = malloc(sizeof(char *) * *argc)))
> +    return -1;

 Please,  virQemuError(VIR_ERR_NO_MEMORY ....) (same or calloc())

> +  printf("Load VM %s\n", file);
> +  if (!(xml = xmlReadDoc(BAD_CAST doc, file ? file : "domain.xml", NULL,
> +                         XML_PARSE_NOENT | XML_PARSE_NONET |
> +                         XML_PARSE_NOERROR | XML_PARSE_NOWARNING))) {
> +    printf("malformed\n");
> +    return NULL;
> +  }

 printf()... it seems you forgot there your debug messages ;-)

> +static void qemudLoadConfig(struct qemud_server *server,
> +			    const char *file) {
> +  FILE *fh;
> +  struct stat st;
> +  struct qemud_vm *vm;
> +  char xml[QEMUD_MAX_XML_LEN];
> +  int ret;
> +
> +  if (!(fh = fopen(file, "r"))) {
> +    return;
> +  }

 No error message?

> +static
> +int qemudBufferAdd(struct qemudBuffer *buf, const char *str) {
> +  int need = strlen(str);
> +
> +static
> +int qemudBufferPrintf(struct qemudBuffer *buf,
> +		      const char *format, ...) {

 Duplicate code?
 
> +  if (qemudBufferPrintf(&buf, "  <uuid>%02x%02x%02x%02x-%02x%02x-%02x%02x-%02x%02x-%02x%02x%02x%02x%02x%02x</uuid>\n",
> +			uuid[0], uuid[1], uuid[2], uuid[3],
> +			uuid[4], uuid[5], uuid[6], uuid[7],
> +			uuid[8], uuid[9], uuid[10], uuid[11],
> +			uuid[12], uuid[13], uuid[14], uuid[15]) < 0)

$ grep "<uuid>" *

test.c:             "
<uuid>%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x</uuid>\n",

xend_internal.c:            virBufferVSprintf(&buf, "
<uuid>%s</uuid>\n", compact);

xml.c:"
<uuid>%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x</uuid>\n",


 UUID.. to be with '-' or not to be with '-'.. that's the question.

 It also seems there is again duplicate code do in test.c and xml.c.

> +      qemudBufferPrintf(&buf, "    <graphics type='vnc'/>\n");

 no format args --> BufferAdd()


    Karel

-- 
 Karel Zak  <kzak at redhat.com>