[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [Qemu-devel] [PATCH v9 2/7] qapi: Introduce add-fd, remove-fd, query-fdsets



I'll send a new version shortly with these updates.

--
Regards,
Corey

On 08/11/2012 10:16 AM, Eric Blake wrote:
On 08/11/2012 07:14 AM, Corey Bryant wrote:
This patch adds support that enables passing of file descriptors
to the QEMU monitor where they will be stored in specified file
descriptor sets.



v9:
  -Use fdset-id rather than fdset_id. (eblake redhat com)
  -Update example for query-fdsets. (eblake redhat com)
  -Close fd immediately on remove-fd.
   (kwolf redhat com, eblake redhat com)
  -Drop fdset refcount, and check dup_fds instead (in a later patch).
   (eblake redhat com)
  -Move mon_refcount code to a later patch. (kwolf redhat com)


+AddfdInfo *qmp_add_fd(bool has_fdset_id, int64_t fdset_id, bool has_opaque,
+                      const char *opaque, Error **errp)
+{
+    int fd;
+    Monitor *mon = cur_mon;
+    MonFdset *mon_fdset;
+    MonFdsetFd *mon_fdset_fd;
+    AddfdInfo *fdinfo;
+
+    fd = qemu_chr_fe_get_msgfd(mon->chr);
+    if (fd == -1) {
+        error_set(errp, QERR_FD_NOT_SUPPLIED);
+        goto error;
+    }
+
+    if (has_fdset_id) {
+        QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
+            if (mon_fdset->id == fdset_id) {
+                break;
+            }
+        }
+        if (mon_fdset == NULL) {
+            error_set(errp, QERR_INVALID_PARAMETER_VALUE, "fdset-id",
+                      "an existing fdset-id or no fdset-id");

The 'no fdset-id' portion of this error message doesn't make sense - it
can only be reached if has_fdset_id was true.

+
+void qmp_remove_fd(int64_t fdset_id, bool has_fd, int64_t fd, Error **errp)
+{
+    MonFdset *mon_fdset;
+    MonFdsetFd *mon_fdset_fd;
+    char fd_str[60];
+
+    QLIST_FOREACH(mon_fdset, &mon_fdsets, next) {
...

+    }
+
+error:
+    snprintf(fd_str, sizeof(fd_str),
+             "fdset-id:%" PRId64 ", fd:%" PRId64, fdset_id, fd);

Oops - fd is uninitialized if has_fd is false and the outer loop failed
to find fdset_id.  You need two separate error messages here, based on
whether fd was provided.

+-> { "execute": "query-fdsets" }
+<- { "return": [
+       {
+         "fds": [
+           {
+             "fd": 30,
+             "opaque": "rdonly:/path/to/file"
+           },
+           {
+             "fd": 24,
+             "opaque": "rdwr:/path/to/file"
+           }
+         ],
+         "fdset-id": 1
+       },
+       {
+         "fds": [
+           {
+             "fd": 28
+           },
+           {
+             "fd": 29
+           }
+         ],
+         "fdset-id": 0
+       },

No trailing comma here.



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]