diff mbox series

[V3,2/2] migration: file URI offset

Message ID 1687466251-310524-3-git-send-email-steven.sistare@oracle.com (mailing list archive)
State New, archived
Headers show
Series migration file URI | expand

Commit Message

Steven Sistare June 22, 2023, 8:37 p.m. UTC
Allow an offset option to be specified as part of the file URI, in
the form "file:filename,offset=offset", where offset accepts the common
size suffixes, or the 0x prefix, but not both.  Migration data is written
to and read from the file starting at offset.  If unspecified, it defaults
to 0.

This is needed by libvirt to store its own data at the head of the file.

Suggested-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 migration/file.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
 qemu-options.hx  |  7 ++++---
 2 files changed, 47 insertions(+), 5 deletions(-)

Comments

Peter Xu June 28, 2023, 5:26 p.m. UTC | #1
On Thu, Jun 22, 2023 at 01:37:31PM -0700, Steve Sistare wrote:
> Allow an offset option to be specified as part of the file URI, in
> the form "file:filename,offset=offset", where offset accepts the common
> size suffixes, or the 0x prefix, but not both.  Migration data is written
> to and read from the file starting at offset.  If unspecified, it defaults
> to 0.
> 
> This is needed by libvirt to store its own data at the head of the file.
> 
> Suggested-by: Daniel P. Berrange <berrange@redhat.com>
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>

Ideally I think we should make "," the separator from the start, but I
suppose no one will try to apply previous patch only, and then start using
a file name contains ",offset=".. so maybe not that a huge deal.

Reviewed-by: Peter Xu <peterx@redhat.com>
Peter Xu June 29, 2023, 9:38 p.m. UTC | #2
On Thu, Jun 22, 2023 at 01:37:31PM -0700, Steve Sistare wrote:
> +static int file_parse_offset(char *filespec, uint64_t *offsetp, Error **errp)
> +{
> +    char *option = strstr(filespec, OFFSET_OPTION);
> +    int ret;
> +
> +    if (option) {
> +        *option = 0;
> +        option += sizeof(OFFSET_OPTION) - 1;
> +        ret = qemu_strtosz(option, NULL, offsetp);
> +        if (ret) {
> +            error_setg_errno(errp, ret, "file URI has bad offset %s", option);

Probably "-ret" here.

> +            return -1;
> +        }
> +    }
> +    return 0;
> +}
Steven Sistare June 30, 2023, 2:25 p.m. UTC | #3
On 6/29/2023 5:38 PM, Peter Xu wrote:
> On Thu, Jun 22, 2023 at 01:37:31PM -0700, Steve Sistare wrote:
>> +static int file_parse_offset(char *filespec, uint64_t *offsetp, Error **errp)
>> +{
>> +    char *option = strstr(filespec, OFFSET_OPTION);
>> +    int ret;
>> +
>> +    if (option) {
>> +        *option = 0;
>> +        option += sizeof(OFFSET_OPTION) - 1;
>> +        ret = qemu_strtosz(option, NULL, offsetp);
>> +        if (ret) {
>> +            error_setg_errno(errp, ret, "file URI has bad offset %s", option);
> 
> Probably "-ret" here.

Yes, thanks.

I have submitted V4 with this fix and your RB's appended.

- Steve
diff mbox series

Patch

diff --git a/migration/file.c b/migration/file.c
index 8e35827..8960be9 100644
--- a/migration/file.c
+++ b/migration/file.c
@@ -6,6 +6,8 @@ 
  */
 
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
+#include "qapi/error.h"
 #include "channel.h"
 #include "file.h"
 #include "migration.h"
@@ -13,14 +15,41 @@ 
 #include "io/channel-util.h"
 #include "trace.h"
 
-void file_start_outgoing_migration(MigrationState *s, const char *filename,
+#define OFFSET_OPTION ",offset="
+
+/* Remove the offset option from @filespec and return it in @offsetp. */
+
+static int file_parse_offset(char *filespec, uint64_t *offsetp, Error **errp)
+{
+    char *option = strstr(filespec, OFFSET_OPTION);
+    int ret;
+
+    if (option) {
+        *option = 0;
+        option += sizeof(OFFSET_OPTION) - 1;
+        ret = qemu_strtosz(option, NULL, offsetp);
+        if (ret) {
+            error_setg_errno(errp, ret, "file URI has bad offset %s", option);
+            return -1;
+        }
+    }
+    return 0;
+}
+
+void file_start_outgoing_migration(MigrationState *s, const char *filespec,
                                    Error **errp)
 {
+    g_autofree char *filename = g_strdup(filespec);
     g_autoptr(QIOChannelFile) fioc = NULL;
+    uint64_t offset = 0;
     QIOChannel *ioc;
 
     trace_migration_file_outgoing(filename);
 
+    if (file_parse_offset(filename, &offset, errp)) {
+        return;
+    }
+
     fioc = qio_channel_file_new_path(filename, O_CREAT | O_WRONLY | O_TRUNC,
                                      0600, errp);
     if (!fioc) {
@@ -28,6 +57,9 @@  void file_start_outgoing_migration(MigrationState *s, const char *filename,
     }
 
     ioc = QIO_CHANNEL(fioc);
+    if (offset && qio_channel_io_seek(ioc, offset, SEEK_SET, errp) < 0) {
+        return;
+    }
     qio_channel_set_name(ioc, "migration-file-outgoing");
     migration_channel_connect(s, ioc, NULL, NULL);
 }
@@ -41,19 +73,28 @@  static gboolean file_accept_incoming_migration(QIOChannel *ioc,
     return G_SOURCE_REMOVE;
 }
 
-void file_start_incoming_migration(const char *filename, Error **errp)
+void file_start_incoming_migration(const char *filespec, Error **errp)
 {
+    g_autofree char *filename = g_strdup(filespec);
     QIOChannelFile *fioc = NULL;
+    uint64_t offset = 0;
     QIOChannel *ioc;
 
     trace_migration_file_incoming(filename);
 
+    if (file_parse_offset(filename, &offset, errp)) {
+        return;
+    }
+
     fioc = qio_channel_file_new_path(filename, O_RDONLY, 0, errp);
     if (!fioc) {
         return;
     }
 
     ioc = QIO_CHANNEL(fioc);
+    if (offset && qio_channel_io_seek(ioc, offset, SEEK_SET, errp) < 0) {
+        return;
+    }
     qio_channel_set_name(QIO_CHANNEL(ioc), "migration-file-incoming");
     qio_channel_add_watch_full(ioc, G_IO_IN,
                                file_accept_incoming_migration,
diff --git a/qemu-options.hx b/qemu-options.hx
index 5aab8fb..5a92210 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4622,7 +4622,7 @@  DEF("incoming", HAS_ARG, QEMU_OPTION_incoming, \
     "                prepare for incoming migration, listen on\n" \
     "                specified protocol and socket address\n" \
     "-incoming fd:fd\n" \
-    "-incoming file:filename\n" \
+    "-incoming file:filename[,offset=offset]\n" \
     "-incoming exec:cmdline\n" \
     "                accept incoming migration on given file descriptor\n" \
     "                or from given external command\n" \
@@ -4641,8 +4641,9 @@  SRST
 ``-incoming fd:fd``
     Accept incoming migration from a given file descriptor.
 
-``-incoming file:filename``
-    Accept incoming migration from a given file.
+``-incoming file:filename[,offset=offset]``
+    Accept incoming migration from a given file starting at offset.
+    offset allows the common size suffixes, or a 0x prefix, but not both.
 
 ``-incoming exec:cmdline``
     Accept incoming migration as an output from specified external