qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH V8 02/39] migration: qemu file wrappers


From: Marc-André Lureau
Subject: Re: [PATCH V8 02/39] migration: qemu file wrappers
Date: Thu, 16 Jun 2022 18:55:04 +0400

Hi

On Wed, Jun 15, 2022 at 6:54 PM Steve Sistare <steven.sistare@oracle.com> wrote:
Add qemu_file_open and qemu_fd_open to create QEMUFile objects for unix
files and file descriptors.

File descriptors are not really unix specific, but that's a detail.

The names of the functions in the summary do not match the code, also details :)

Eventually, I would suggest to follow the libc fopen/fdopen naming, if that makes sense. (or the QIOChannel naming)


Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 migration/qemu-file-channel.c | 36 ++++++++++++++++++++++++++++++++++++
 migration/qemu-file-channel.h |  6 ++++++
 2 files changed, 42 insertions(+)

diff --git a/migration/qemu-file-channel.c b/migration/qemu-file-channel.c
index bb5a575..cc5aebc 100644
--- a/migration/qemu-file-channel.c
+++ b/migration/qemu-file-channel.c
@@ -27,8 +27,10 @@
 #include "qemu-file.h"
 #include "io/channel-socket.h"
 #include "io/channel-tls.h"
+#include "io/channel-file.h"
 #include "qemu/iov.h"
 #include "qemu/yank.h"
+#include "qapi/error.h"
 #include "yank_functions.h"


@@ -192,3 +194,37 @@ QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc)
     object_ref(OBJECT(ioc));
     return qemu_fopen_ops(ioc, &channel_output_ops, true);
 }
+
+QEMUFile *qemu_fopen_file(const char *path, int flags, int mode,
+                          const char *name, Error **errp)
+{

I would add ERRP_GUARD();
 
+    g_autoptr(QIOChannelFile) fioc = NULL;
+    QIOChannel *ioc;
+    QEMUFile *f;
+
+    if (flags & O_RDWR) {
+        error_setg(errp, "qemu_fopen_file %s: O_RDWR not supported", path);
+        return NULL;
+    }

Why not take a "bool writable" instead, like the fdopen below?
 
+
+    fioc = qio_channel_file_new_path(path, flags, mode, errp);
+    if (!fioc) {
+        return NULL;
+    }
+
+    ioc = QIO_CHANNEL(fioc);
+    qio_channel_set_name(ioc, name);
+    f = (flags & O_WRONLY) ? qemu_fopen_channel_output(ioc) :
+                             qemu_fopen_channel_input(ioc);
+    return f;

"f" and parentheses are kinda superfluous
 
+}
+
+QEMUFile *qemu_fopen_fd(int fd, bool writable, const char *name)
+{
+    g_autoptr(QIOChannelFile) fioc = qio_channel_file_new_fd(fd);
+    QIOChannel *ioc = QIO_CHANNEL(fioc);
+    QEMUFile *f = writable ? qemu_fopen_channel_output(ioc) :
+                             qemu_fopen_channel_input(ioc);
+    qio_channel_set_name(ioc, name);
+    return f;

or:

g_autoptr(QIOChannelFile) fioc = qio_channel_file_new_fd(fd);
qio_channel_set_name(QIO_CHANNEL(fioc), name);
return writable ? qemu_fopen_channel_output(ioc) : qemu_fopen_channel_input(ioc);
 
+}
diff --git a/migration/qemu-file-channel.h b/migration/qemu-file-channel.h
index 0028a09..75fd0ad 100644
--- a/migration/qemu-file-channel.h
+++ b/migration/qemu-file-channel.h
@@ -29,4 +29,10 @@

 QEMUFile *qemu_fopen_channel_input(QIOChannel *ioc);
 QEMUFile *qemu_fopen_channel_output(QIOChannel *ioc);
+
+QEMUFile *qemu_fopen_file(const char *path, int flags, int mode,
+                         const char *name, Error **errp);
+
+QEMUFile *qemu_fopen_fd(int fd, bool writable, const char *name);
+
 #endif
--
1.8.3.1




--
Marc-André Lureau

reply via email to

[Prev in Thread] Current Thread [Next in Thread]