On 08/09/2012 03:04 AM, Stefan Hajnoczi wrote:
On Tue, Aug 7, 2012 at 4:58 PM, Corey Bryant <address@hidden> wrote:
+##
+# @FdsetFdInfo:
+#
+# Information about a file descriptor that belongs to an fd set.
+#
+# @fd: The file descriptor value.
+#
+# @removed: If true, the remove-fd command has been issued for this fd.
+#
+# Since: 1.2.0
+##
+{ 'type': 'FdsetFdInfo', 'data': {'fd': 'int', 'removed': 'bool'} }
I'm not sure if the removed field is useful, especially since
remove-fd is idempotent (you can keep querying fds and then marking
them removed again until they finally close). The reason I suggest
minimizing the schema is so that we can change implementation details
later without having to synthesize this state.
Pretty much agreed - rather than listing 'removed', omitting the fd by
default will match the monitors expectations ("why are you telling me
about an fd I told you to remove?"). The only reason I could see for
keeping it would be for debug purposes, but that would be debugging of
qemu, not the application connected to the monitor, at which point gdb
debugging is probably better.
+{ 'type': 'FdsetInfo',
+ 'data': {'fdset_id': 'int', 'refcount': 'int', 'in_use': 'bool',
+ 'fds': ['FdsetFdInfo']} }
Why are refcount and in_use exposed? How will applications use them?
This seems like internal state to me.
refcount _might_ be useful for knowing whether qemu is actively using
the fdset. For example, in the sequence:
add-fd nnn
drive-add
if libvirtd crashes after sending drive-add but before getting a
response, then on restart it has to figure out if the drive-add took or
failed. A non-zero refcount means it took. But then again, so does a
query-block. I like the approach of being minimal until we prove we
need it, and can't right now think of anything where having a refcount
would tell libvirt anything new that it can't already learn from
somewhere else.
Should add-fd allow the application to associate an opaque string with
the fdset? This could be used to recover after crash. Otherwise the
application needs to store the fdset_id -> filename mapping in a file
on disk and hope it was safely stored before crash. It seems like the
best place to keep this info is with the fdset.
Very nice idea! In fact, even nicer than returning a markup of O_RDONLY
- if the management app cares about knowing details on an fd, such as
whether it is read-only, then an opaque string tied to the fd in the
fdset becomes very useful. The string needs to be optional on add-fd.
(Libvirt might even use an xml-like string like "<fd
file='/path/to/file' mode='O_RDONLY'/>", but of course, being an opaque
string, qemu doesn't have to care what the string contains.)