[Qemu-devel] getfd monitor command broken

[Qemu-devel] getfd monitor command broken

[Ed Swierk]

Commit c62313bbdc48f72e93fa8196f2fff96ba35e4e9d seems to have broken
the getfd monitor command in qemu 0.12.

tcp_chr_read() calls tcp_chr_recv(), which checks whether the received
message includes an SCM_RIGHTS header, and if so, stores the received
fd in the CharDriverState struct. tcp_chr_read() passes the received
data to the monitor via qemu_chr_read(), and then closes the stored
fd.

Previously, tcp_chr_read() would receive the entire getfd command in
one message (perhaps by sheer luck), allowing the monitor to call
qemu_chr_get_msgfd() to obtain the stored fd before it disappeared.
Now that tcp_chr_read() receives only 1 byte at a time, the stored fd
vanishes when it receives byte 2.

I'm too confused by this control flow to suggest a proper solution,
but commenting out the "if (s->msgfd != -1)" block in tcp_chr_read()
at least makes the problem go away.

--Ed

Re: [Qemu-devel] getfd monitor command broken

[Luiz Capitulino]

On Fri, 19 Feb 2010 10:21:41 -0800
Ed Swierk <eswierk@aristanetworks.com> wrote:

> Commit c62313bbdc48f72e93fa8196f2fff96ba35e4e9d seems to have broken
> the getfd monitor command in qemu 0.12.

 Does it work with current master? How do you reproduce it?

Re: [Qemu-devel] getfd monitor command broken

[Ed Swierk]

On Mon, Feb 22, 2010 at 12:51 PM, Luiz Capitulino
<lcapitulino@redhat.com> wrote:
> On Fri, 19 Feb 2010 10:21:41 -0800
> Ed Swierk <eswierk@aristanetworks.com> wrote:
>
>> Commit c62313bbdc48f72e93fa8196f2fff96ba35e4e9d seems to have broken
>> the getfd monitor command in qemu 0.12.
>
>  Does it work with current master? How do you reproduce it?

I have tried qemu-kvm 0.12.1.2, but not current qemu master.

I found the problem by using the attach-interface command in virsh to
add a NIC device and add a host network via the monitor.

Unfortunately libvirt is the only application I know of that uses the
getfd command, but maybe I'll try hacking together a little test
program.

--Ed

Re: [Qemu-devel] getfd monitor command broken

[Ed Swierk]

[-- Attachment #1: Type: text/plain, Size: 424 bytes --]

On Mon, Feb 22, 2010 at 12:51 PM, Luiz Capitulino
<lcapitulino@redhat.com> wrote:
> How do you reproduce it?

Here's a test program that reproduces the problem. Start qemu with

  -chardev socket,id=monitor,path=/tmp/qemu-monitor,server,nowait -mon
chardev=monitor,mode=readline

and run check_getfd /tmp/qemu-monitor. It will print an error and
return nonzero if the monitor output indicates getfd or closefd
failed.

--Ed

[-- Attachment #2: check_getfd.c --]
[-- Type: text/x-csrc, Size: 3484 bytes --]

/*
 * check_getfd
 *
 * Tests the qemu getfd monitor command
 *
 * Copyright (c) 2010 Arista Networks, Inc.
 * 
 * Permission is hereby granted, free of charge, to any person obtaining a copy
 * of this software and associated documentation files (the "Software"), to deal
 * in the Software without restriction, including without limitation the rights
 * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
 * copies of the Software, and to permit persons to whom the Software is
 * furnished to do so, subject to the following conditions:
 *
 * The above copyright notice and this permission notice shall be included in
 * all copies or substantial portions of the Software.
 *
 * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
 * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
 * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
 * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
 * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 * THE SOFTWARE.
 */

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <fcntl.h>

#include <sys/types.h>
#include <sys/socket.h>
#include <sys/un.h>

int receive_output(int s, char *m) {
   unsigned int i = 0;
   char buf[10240];

   buf[0] = '\0';
   while (1) {
      if (recv(s, &buf[i], 1, 0) < 0) {
         perror("Failed to receive");
         return -1;
      }
      buf[++i] = '\0';
      if ((i > 7) && !strcmp(&buf[i-7], "(qemu) "))
         break;
   }

   if (m &&
       ((i < strlen(m) + 7) || strncmp(&buf[i-7-strlen(m)], m, strlen(m)))) {
      fprintf(stderr, "%s\n", buf);
      return -1;
   }

   return 0;
}

int main(int argc, char *argv[]) {
   struct sockaddr_un addr;
   int s;
   int fd;
   char fdbuf[CMSG_SPACE(sizeof(fd))];
   struct msghdr msg;
   struct cmsghdr *cmsg;
   struct iovec mvec;
   char *cmd = "getfd MYFD\nclosefd MYFD\n";

   if (argc != 2) {
      printf("Usage: %s QEMU_MONITOR\n\n", argv[0]);
      printf("  (start qemu with -chardev socket,id=monitor,path=QEMU_MONITOR"
             ",server,nowait -mon chardev=monitor,mode=readline)\n");
      return 1;
   }

   fd = open("/dev/null", O_RDWR);
   if (fd < 0) {
      perror("Failed to open /dev/null");
      return 1;
   }

   memset(&addr, 0, sizeof(addr));
   addr.sun_family = AF_UNIX;
   strncpy(addr.sun_path, argv[1], sizeof(addr.sun_path));

   s = socket(PF_UNIX, SOCK_STREAM, 0);
   if (s < 0) {
      perror("No socket");
      return 1;
   }

   if (connect(s, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
      perror("Failed to connect");
      return 1;
   }

   if (receive_output(s, NULL) < 0)
      return 1;

   mvec.iov_base = cmd;
   mvec.iov_len = strlen(cmd) + 1;
   msg.msg_name = NULL;
   msg.msg_namelen = 0;
   msg.msg_iov = &mvec;
   msg.msg_iovlen = 1;
   msg.msg_control = fdbuf;
   msg.msg_controllen = CMSG_LEN(sizeof(fd));
   msg.msg_flags = 0;

   cmsg = CMSG_FIRSTHDR(&msg);
   cmsg->cmsg_level = SOL_SOCKET;
   cmsg->cmsg_type = SCM_RIGHTS;
   cmsg->cmsg_len = msg.msg_controllen;
   memcpy(CMSG_DATA(cmsg), &fd, sizeof(fd));
   
   if (sendmsg(s, &msg, 0) < 0) {
      perror("Failed to send");
      return 1;
   }

   if (receive_output(s, "\033[K\r\n") < 0)
      return 1;
   if (receive_output(s, "\033[K\r\n") < 0)
      return 1;

   return 0;
}