[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH V8 02/39] migration: qemu file wrappers
From: |
Steven Sistare |
Subject: |
Re: [PATCH V8 02/39] migration: qemu file wrappers |
Date: |
Tue, 5 Jul 2022 14:25:45 -0400 |
User-agent: |
Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 |
On 6/16/2022 11:29 AM, Daniel P. Berrangé wrote:
> On Wed, Jun 15, 2022 at 07:51:49AM -0700, Steve Sistare wrote:
>> Add qemu_file_open and qemu_fd_open to create QEMUFile objects for unix
>> files and file descriptors.
>>
>> 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)
>> +{
>> + g_autoptr(QIOChannelFile) fioc = NULL;
>> + QIOChannel *ioc;
>> + QEMUFile *f;
>> +
>> + if (flags & O_RDWR) {
>
> IIRC, O_RDWR may expand to more than 1 bit, so needs a strict
> equality test
>
> if ((flags & O_RDWR) == O_RDWR)
Hmm, on what OS? No harm if I just do it, but the next reviewer will tell
me to remove the unnecessary equality test :)
>> + error_setg(errp, "qemu_fopen_file %s: O_RDWR not supported", path);
>> + return NULL;
>> + }
>> +
>> + 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;
>> +}
>> +
>> +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;
>> +}
>> 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);
>
> Note we used the explicit names "_input" and "_output" in
> the existing methods as they're more readable in the calling
> sides than "true" / "false".
>
> Similarly we had qemu_open vs qemu_create, so that we don't
> have the ambiguity of whuether 'mode' is needed or not. IOW,
> I'd suggest we have
>
> QEMUFile *qemu_fopen_file_output(const char *path, int mode,
> const char *name, Error **errp);
> QEMUFile *qemu_fopen_file_input(const char *path,
> const char *name, Error **errp);
>
> QEMUFile *qemu_fopen_fd_input(int fd, const char *name);
> QEMUFile *qemu_fopen_fd_output(int fd, const char *name);
Will do. I do need the flags argument in the fopen_file calls, though.
- Steve