diff mbox

[RFC,RDMA,support,v5:,07/12] additional savevm.c accessors for RDMA

Message ID 1365476681-31593-8-git-send-email-mrhines@linux.vnet.ibm.com
State New
Headers show

Commit Message

mrhines@linux.vnet.ibm.com April 9, 2013, 3:04 a.m. UTC 
From: "Michael R. Hines" <mrhines@us.ibm.com>

1. qemu_file_ops_are()
2. qemu_file_update_position()    (for f->pos)

Also need to be here:
rdma_read_ops
rdma_write_ops

Signed-off-by: Michael R. Hines <mrhines@us.ibm.com>
---
 savevm.c |   57 ++++++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 48 insertions(+), 9 deletions(-)

Comments

Paolo Bonzini April 9, 2013, 5:03 p.m. UTC | #1 
Il 09/04/2013 05:04, mrhines@linux.vnet.ibm.com ha scritto:
> From: "Michael R. Hines" <mrhines@us.ibm.com>
> 
> 1. qemu_file_ops_are()
> 2. qemu_file_update_position()    (for f->pos)
> 
> Also need to be here:
> rdma_read_ops
> rdma_write_ops
> 
> Signed-off-by: Michael R. Hines <mrhines@us.ibm.com>
> ---
>  savevm.c |   57 ++++++++++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 48 insertions(+), 9 deletions(-)

As agreed before, please return the page size from save_ram_page and
update position in savevm.c.

Add enough QEMUFileOps, and you won't need qemu_file_ops_are.

Paolo
Peter Maydell April 9, 2013, 5:31 p.m. UTC | #2 
On 9 April 2013 04:04,  <mrhines@linux.vnet.ibm.com> wrote:
> From: "Michael R. Hines" <mrhines@us.ibm.com>
>
> 1. qemu_file_ops_are()
> 2. qemu_file_update_position()    (for f->pos)
>
> Also need to be here:
> rdma_read_ops
> rdma_write_ops

Do you think you could try to expand on your commit messages
a bit? The idea is that a commit message should generally
give an overview of the patch including rationale; it should
be reasonably meaningful if you look only at the commit
message and not the patch itself. This one has a very
abbreviated description of the "what" and is missing any
kind of "why".

thanks
-- PMM
mrhines@linux.vnet.ibm.com April 9, 2013, 6:04 p.m. UTC | #3 
On 04/09/2013 01:31 PM, Peter Maydell wrote:
> On 9 April 2013 04:04,  <mrhines@linux.vnet.ibm.com> wrote:
>> From: "Michael R. Hines" <mrhines@us.ibm.com>
>>
>> 1. qemu_file_ops_are()
>> 2. qemu_file_update_position()    (for f->pos)
>>
>> Also need to be here:
>> rdma_read_ops
>> rdma_write_ops

My apologies..... will do =)

> Do you think you could try to expand on your commit messages
> a bit? The idea is that a commit message should generally
> give an overview of the patch including rationale; it should
> be reasonably meaningful if you look only at the commit
> message and not the patch itself. This one has a very
> abbreviated description of the "what" and is missing any
> kind of "why".
>
> thanks
> -- PMM
>
diff mbox

Patch

diff --git a/savevm.c b/savevm.c
index b1d8988..0f5c7aa 100644
--- a/savevm.c
+++ b/savevm.c
@@ -32,6 +32,7 @@ 
 #include "qemu/timer.h"
 #include "audio/audio.h"
 #include "migration/migration.h"
+#include "migration/rdma.h"
 #include "qemu/sockets.h"
 #include "qemu/queue.h"
 #include "sysemu/cpus.h"
@@ -409,16 +410,24 @@  static const QEMUFileOps socket_write_ops = {
     .close =      socket_close
 };
 
-QEMUFile *qemu_fopen_socket(int fd, const char *mode)
+bool qemu_file_mode_is_not_valid(const char * mode)
 {
-    QEMUFileSocket *s = g_malloc0(sizeof(QEMUFileSocket));
-
     if (mode == NULL ||
         (mode[0] != 'r' && mode[0] != 'w') ||
         mode[1] != 'b' || mode[2] != 0) {
         fprintf(stderr, "qemu_fopen: Argument validity check failed\n");
-        return NULL;
+        return true;
     }
+    
+    return false;
+}
+
+QEMUFile *qemu_fopen_socket(int fd, const char *mode)
+{
+    QEMUFileSocket *s = g_malloc0(sizeof(QEMUFileSocket));
+
+    if(qemu_file_mode_is_not_valid(mode))
+	return NULL;
 
     s->fd = fd;
     if (mode[0] == 'w') {
@@ -430,16 +439,27 @@  QEMUFile *qemu_fopen_socket(int fd, const char *mode)
     return s->file;
 }
 
+/*
+ * These have to be here for qemu_file_ops_are()
+ * The function pointers compile to NULL if
+ * RDMA is disabled at configure time. 
+ */
+const QEMUFileOps rdma_read_ops = {
+    .get_buffer = qemu_rdma_get_buffer,
+    .close =      qemu_rdma_close,
+};
+
+const QEMUFileOps rdma_write_ops = {
+    .put_buffer = qemu_rdma_put_buffer,
+    .close =      qemu_rdma_close,
+};
+
 QEMUFile *qemu_fopen(const char *filename, const char *mode)
 {
     QEMUFileStdio *s;
 
-    if (mode == NULL ||
-	(mode[0] != 'r' && mode[0] != 'w') ||
-	mode[1] != 'b' || mode[2] != 0) {
-        fprintf(stderr, "qemu_fopen: Argument validity check failed\n");
+    if(qemu_file_mode_is_not_valid(mode))
         return NULL;
-    }
 
     s = g_malloc0(sizeof(QEMUFileStdio));
 
@@ -790,6 +810,17 @@  int qemu_get_byte(QEMUFile *f)
     return result;
 }
 
+/*
+ * Validate which operations are actually in use
+ * before attempting to access opaque data.
+ */
+void * qemu_file_ops_are(QEMUFile *f, const QEMUFileOps *ops)
+{
+    if (f->ops == ops)
+        return f->opaque;
+    return NULL;
+}
+
 int64_t qemu_ftell(QEMUFile *f)
 {
     qemu_fflush(f);
@@ -807,6 +838,14 @@  int qemu_file_rate_limit(QEMUFile *f)
     return 0;
 }
 
+/*
+ * For users, like RDMA, that don't go through the QEMUFile buffer directly.
+ */
+void qemu_file_update_position(QEMUFile *f, int64_t inc)
+{
+    f->pos += inc;
+}
+
 int64_t qemu_file_get_rate_limit(QEMUFile *f)
 {
     return f->xfer_limit;