Re: [Qemu-devel] [PATCH 1/7] vnvram: VNVRAM bdrv support
Am 23.05.2013 um 19:44 hat Corey Bryant geschrieben:
Provides low-level VNVRAM functionality that reads and writes data,
such as an entry's binary blob, to a drive image using the block
driver.
Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com
+/*
+ * Increase the drive size if it's too small to fit the VNVRAM data
+ */
+static int vnvram_drv_adjust_size(VNVRAM *vnvram)
+{
+int rc = 0;
+int64_t needed_size;
+
+needed_size = 0;
+
+if (bdrv_getlength(vnvram-bds) needed_size) {
+rc = bdrv_truncate(vnvram-bds, needed_size);
+if (rc != 0) {
+DPRINTF(%s: VNVRAM drive too small\n, __func__);
+}
+}
+
+return rc;
+}
This function doesn't make a whole lot of sense. It truncates the file
to size 0 if and only if bdrv_getlength() returns an error.
+
+/*
+ * Write a header to the drive with entry count of zero
+ */
+static int vnvram_drv_hdr_create_empty(VNVRAM *vnvram)
+{
+VNVRAMDrvHdr hdr;
+
+hdr.version = VNVRAM_CURRENT_VERSION;
+hdr.magic = VNVRAM_MAGIC;
+hdr.num_entries = 0;
+
+vnvram_drv_hdr_cpu_to_be((hdr));
+
+if (bdrv_pwrite(vnvram-bds, 0, (hdr), sizeof(hdr)) != sizeof(hdr)) {
+DPRINTF(%s: Write of header to drive failed\n, __func__);
+return -EIO;
+}
+
+vnvram-end_offset = sizeof(VNVRAMDrvHdr);
+
+return 0;
+}
+
+/*
+ * Read the header from the drive
+ */
+static int vnvram_drv_hdr_read(VNVRAM *vnvram, VNVRAMDrvHdr *hdr)
+{
+if (bdrv_pread(vnvram-bds, 0, hdr, sizeof(*hdr)) != sizeof(*hdr)) {
+DPRINTF(%s: Read of header from drive failed\n, __func__);
+return -EIO;
+}
Why do you turn all errors into -EIO instead of returning the real error
code? (More instances of the same thing follow)
+
+vnvram_drv_hdr_be_to_cpu(hdr);
+
+return 0;
+}
+}
Kevin
Re: [Qemu-devel] [PATCH 1/7] vnvram: VNVRAM bdrv support
On 05/24/2013 09:06 AM, Kevin Wolf wrote:
Am 23.05.2013 um 19:44 hat Corey Bryant geschrieben:
Provides low-level VNVRAM functionality that reads and writes data,
such as an entry's binary blob, to a drive image using the block
driver.
Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com
+/*
+ * Increase the drive size if it's too small to fit the VNVRAM data
+ */
+static int vnvram_drv_adjust_size(VNVRAM *vnvram)
+{
+int rc = 0;
+int64_t needed_size;
+
+needed_size = 0;
+
+if (bdrv_getlength(vnvram-bds) needed_size) {
+rc = bdrv_truncate(vnvram-bds, needed_size);
+if (rc != 0) {
+DPRINTF(%s: VNVRAM drive too small\n, __func__);
+}
+}
+
+return rc;
+}
This function doesn't make a whole lot of sense. It truncates the file
to size 0 if and only if bdrv_getlength() returns an error.
There's a later patch that adds a get size function and changes the
initialization of needed_size to the actual size needed to store VNVRAM
data. Anyway I should probably just include that change in this patch.
I think I'll still need this function or part of it with the new
simplified approach that it looks like we're going to take.
+
+/*
+ * Write a header to the drive with entry count of zero
+ */
+static int vnvram_drv_hdr_create_empty(VNVRAM *vnvram)
+{
+VNVRAMDrvHdr hdr;
+
+hdr.version = VNVRAM_CURRENT_VERSION;
+hdr.magic = VNVRAM_MAGIC;
+hdr.num_entries = 0;
+
+vnvram_drv_hdr_cpu_to_be((hdr));
+
+if (bdrv_pwrite(vnvram-bds, 0, (hdr), sizeof(hdr)) != sizeof(hdr)) {
+DPRINTF(%s: Write of header to drive failed\n, __func__);
+return -EIO;
+}
+
+vnvram-end_offset = sizeof(VNVRAMDrvHdr);
+
+return 0;
+}
+
+/*
+ * Read the header from the drive
+ */
+static int vnvram_drv_hdr_read(VNVRAM *vnvram, VNVRAMDrvHdr *hdr)
+{
+if (bdrv_pread(vnvram-bds, 0, hdr, sizeof(*hdr)) != sizeof(*hdr)) {
+DPRINTF(%s: Read of header from drive failed\n, __func__);
+return -EIO;
+}
Why do you turn all errors into -EIO instead of returning the real error
code? (More instances of the same thing follow)
Good point, there's no reason to mask the original error code.
+
+vnvram_drv_hdr_be_to_cpu(hdr);
+
+return 0;
+}
+}
Kevin
--
Regards,
Corey Bryant
Re: [Qemu-devel] [PATCH 1/7] vnvram: VNVRAM bdrv support
Am 24.05.2013 um 17:33 hat Corey Bryant geschrieben:
On 05/24/2013 09:06 AM, Kevin Wolf wrote:
Am 23.05.2013 um 19:44 hat Corey Bryant geschrieben:
Provides low-level VNVRAM functionality that reads and writes data,
such as an entry's binary blob, to a drive image using the block
driver.
Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com
+/*
+ * Increase the drive size if it's too small to fit the VNVRAM data
+ */
+static int vnvram_drv_adjust_size(VNVRAM *vnvram)
+{
+int rc = 0;
+int64_t needed_size;
+
+needed_size = 0;
+
+if (bdrv_getlength(vnvram-bds) needed_size) {
+rc = bdrv_truncate(vnvram-bds, needed_size);
+if (rc != 0) {
+DPRINTF(%s: VNVRAM drive too small\n, __func__);
+}
+}
+
+return rc;
+}
This function doesn't make a whole lot of sense. It truncates the file
to size 0 if and only if bdrv_getlength() returns an error.
There's a later patch that adds a get size function and changes
the initialization of needed_size to the actual size needed to store
VNVRAM data. Anyway I should probably just include that change in
this patch. I think I'll still need this function or part of it
with the new simplified approach that it looks like we're going to
take.
Okay. But even then, do you really want to truncate on errors?
Kevin
Re: [Qemu-devel] [PATCH 1/7] vnvram: VNVRAM bdrv support
On 05/24/2013 11:37 AM, Kevin Wolf wrote:
Am 24.05.2013 um 17:33 hat Corey Bryant geschrieben:
On 05/24/2013 09:06 AM, Kevin Wolf wrote:
Am 23.05.2013 um 19:44 hat Corey Bryant geschrieben:
Provides low-level VNVRAM functionality that reads and writes data,
such as an entry's binary blob, to a drive image using the block
driver.
Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com
+/*
+ * Increase the drive size if it's too small to fit the VNVRAM data
+ */
+static int vnvram_drv_adjust_size(VNVRAM *vnvram)
+{
+int rc = 0;
+int64_t needed_size;
+
+needed_size = 0;
+
+if (bdrv_getlength(vnvram-bds) needed_size) {
+rc = bdrv_truncate(vnvram-bds, needed_size);
+if (rc != 0) {
+DPRINTF(%s: VNVRAM drive too small\n, __func__);
+}
+}
+
+return rc;
+}
This function doesn't make a whole lot of sense. It truncates the file
to size 0 if and only if bdrv_getlength() returns an error.
There's a later patch that adds a get size function and changes
the initialization of needed_size to the actual size needed to store
VNVRAM data. Anyway I should probably just include that change in
this patch. I think I'll still need this function or part of it
with the new simplified approach that it looks like we're going to
take.
Okay. But even then, do you really want to truncate on errors?
Kevin
True, it'll need something to account for bdrv_getlength() failures and
not truncate in that case.
--
Regards,
Corey Bryant
[Qemu-devel] [PATCH 1/7] vnvram: VNVRAM bdrv support
Provides low-level VNVRAM functionality that reads and writes data,
such as an entry's binary blob, to a drive image using the block
driver.
Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com
---
Makefile.objs |2 +
vnvram.c | 487 +
vnvram.h | 22 +++
3 files changed, 511 insertions(+), 0 deletions(-)
create mode 100644 vnvram.c
create mode 100644 vnvram.h
diff --git a/Makefile.objs b/Makefile.objs
index 286ce06..4875a94 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -76,6 +76,8 @@ common-obj-$(CONFIG_SECCOMP) += qemu-seccomp.o
common-obj-$(CONFIG_SMARTCARD_NSS) += $(libcacard-y)
+common-obj-y += vnvram.o
+
##
# qapi
diff --git a/vnvram.c b/vnvram.c
new file mode 100644
index 000..e467198
--- /dev/null
+++ b/vnvram.c
@@ -0,0 +1,487 @@
+/*
+ * VNVRAM -- stores persistent data in image files
+ *
+ * Copyright (C) 2013 IBM Corporation
+ *
+ * Authors:
+ * Stefan Bergerstef...@us.ibm.com
+ * Corey Bryant cor...@linux.vnet.ibm.com
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include vnvram.h
+#include block/block.h
+
+/*
+#define VNVRAM_DEBUG
+*/
+
+#ifdef VNVRAM_DEBUG
+#define DPRINTF(fmt, ...) \
+do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) \
+do { } while (0)
+#endif
+
+#define VNVRAM_ENTRY_DATA \
+VNVRAMEntryName name; /* name of entry */ \
+uint64_t blob_offset; /* start of blob on drive */ \
+uint32_t cur_size;/* current size of blob */ \
+uint32_t max_size;/* max size of blob */
+
+/* The following VNVRAM information is stored in-memory */
+typedef struct VNVRAMEntry {
+VNVRAM_ENTRY_DATA
+QLIST_ENTRY(VNVRAMEntry) next;
+} VNVRAMEntry;
+
+struct VNVRAM {
+char *drv_id;/* corresponds to -drive id= on command line */
+BlockDriverState *bds; /* bds for the VNVRAM drive */
+uint64_t end_offset; /* offset on drive where next entry will go */
+QLIST_HEAD(entries_head, VNVRAMEntry) entries_head; /* in-memory entries */
+QLIST_ENTRY(VNVRAM) list;
+};
+
+/* There can be multiple VNVRAMS */
+static QLIST_HEAD(, VNVRAM) vnvrams = QLIST_HEAD_INITIALIZER(vnvrams);
+
+#define VNVRAM_VERSION_11
+#define VNVRAM_CURRENT_VERSION VNVRAM_VERSION_1
+#define VNVRAM_MAGIC0x4E56524D /* NVRM */
+
+/* VNVRAM drive data consists of a header followed by entries and their blobs.
+ * For example:
+ * | header | entry 1 | entry 1's blob | entry 2 | entry 2's blob | ... |
+ */
+typedef struct VNVRAMDrvHdr {
+uint16_t version;
+uint32_t magic;
+uint32_t num_entries;
+} QEMU_PACKED VNVRAMDrvHdr;
+
+typedef struct VNVRAMDrvEntry {
+VNVRAM_ENTRY_DATA
+} QEMU_PACKED VNVRAMDrvEntry;
+
+static int vnvram_drv_entry_create(VNVRAM *, VNVRAMEntry *, uint64_t,
uint32_t);
+static int vnvram_drv_entry_update(VNVRAM *, VNVRAMEntry *, uint64_t,
uint32_t);
+
+/*
+ * Macros for finding entries and their drive offsets
+ */
+#define VNVRAM_FIRST_ENTRY(vnvram) \
+QLIST_FIRST((vnvram)-entries_head)
+
+#define VNVRAM_NEXT_ENTRY(cur_entry) \
+QLIST_NEXT(cur_entry, next)
+
+#define VNVRAM_FIRST_ENTRY_OFFSET() \
+sizeof(VNVRAMDrvHdr)
+
+#define VNVRAM_NEXT_ENTRY_OFFSET(entry) \
+((entry)-blob_offset + (entry)-max_size)
+
+#define VNVRAM_NEXT_AVAIL_BLOB_OFFSET(vnvram) \
+((vnvram)-end_offset + sizeof(VNVRAMDrvEntry))
+
+#define VNVRAM_ENTRY_OFFSET_FROM_BLOB(blob_offset) \
+(blob_offset - sizeof(VNVRAMDrvEntry))
+
+#define VNVRAM_BLOB_OFFSET_FROM_ENTRY(entry_offset) \
+(entry_offset + sizeof(VNVRAMDrvEntry))
+
+/* VNVRAM drv /
+/* Low-level VNVRAM functions that work with the drive header and*/
+/* entries. */
+/*/
+
+/*
+ * Big-endian conversions
+ */
+static void vnvram_drv_hdr_cpu_to_be(VNVRAMDrvHdr *hdr)
+{
+hdr-version = cpu_to_be16(hdr-version);
+hdr-magic = cpu_to_be32(hdr-magic);
+hdr-num_entries = cpu_to_be32(hdr-num_entries);
+}
+
+static void vnvram_drv_hdr_be_to_cpu(VNVRAMDrvHdr *hdr)
+{
+hdr-version = be16_to_cpu(hdr-version);
+hdr-magic = be32_to_cpu(hdr-magic);
+hdr-num_entries = be32_to_cpu(hdr-num_entries);
+}
+
+static void vnvram_drv_entry_cpu_to_be(VNVRAMDrvEntry *drv_entry)
+{
+drv_entry-blob_offset = cpu_to_be64(drv_entry-blob_offset);
+drv_entry-cur_size = cpu_to_be32(drv_entry-cur_size);
+drv_entry-max_size = cpu_to_be32(drv_entry-max_size);
+}
+
+static void vnvram_drv_entry_be_to_cpu(VNVRAMDrvEntry *drv_entry)
+{
+drv_entry-blob_offset = be64_to_cpu(drv_entry-blob_offset);
+drv_entry-cur_size =
