Re: [Qemu-devel] [PATCH 1/1] Added address_space_init2().
Hi,
On 01/07/2013 03:05 PM, Andreas Färber wrote:
A memory: prefix in the subject would've been nice for filtering.
OK
Am 07.01.2013 13:07, schrieb Alexander Barabash:
address_space_init2: initializes a named address space.
What for? There are no users in this patch that justify its utility over
setting the field manually.
Names of memory regions and address spaces are used for debugging purposes.
Currently, there is no way for address spaces other than memory and
io to
appear in debugging output.
This is used in some code to be posted later.
The name is really awful, maybe ..._init_with_name or add a name
argument to the existing function and let the existing callers pass NULL?
Changing the existing callers of address_space_init() is OK with me.
I did not do that primarily because that would be an obvious way to do
to start with. And since this was not implemented this way,
there should have been some some reason.
However, I'll send a PATCH along these lines.
Regards,
Andreas
Regards,
Alex
Signed-off-by: Alexander Barabash alexander_barab...@mentor.com
---
include/exec/memory.h |9 +
memory.c |6 ++
2 files changed, 15 insertions(+)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 2322732..8f8a31d 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -820,6 +820,15 @@ void mtree_info(fprintf_function mon_printf, void *f);
*/
void address_space_init(AddressSpace *as, MemoryRegion *root);
+/**
+ * address_space_init2: initializes a named address space
+ *
+ * @as: an uninitialized #AddressSpace
+ * @root: a #MemoryRegion that routes addesses for the address space
+ * @name: used for debugging
+ */
+void address_space_init2(AddressSpace *as, MemoryRegion *root,
+ const char *name);
/**
* address_space_destroy: destroy an address space
diff --git a/memory.c b/memory.c
index 410c5f8..1652c10 100644
--- a/memory.c
+++ b/memory.c
@@ -1574,6 +1574,12 @@ void address_space_init(AddressSpace *as, MemoryRegion
*root)
address_space_init_dispatch(as);
}
+void address_space_init2(AddressSpace *as, MemoryRegion *root, const char *name)
+{
+address_space_init(as, root);
+as-name = g_strdup(name);
+}
+
void address_space_destroy(AddressSpace *as)
{
/* Flush out anything from MemoryListeners listening in on this */
Re: [Qemu-devel] [PATCH 1/1] Support abstract socket namespace in AF_UNIX socket family.
Hi,
On 01/07/2013 04:07 PM, Anthony Liguori wrote:
Alexander Barabash alexander_barab...@mentor.com writes:
The abstract socket namespace is a nonportable Linux extension.
The sockets' names in this namespace have no connection
with file system pathnames. To specify a named AF_UNIX socket
in the abstract socket namespace, start the socket's path option
with a backslash followed by zero: \0.
Signed-off-by: Alexander Barabash alexander_barab...@mentor.com
Magic interpretation of path names is a bad thing in general.
I think it would be better to add a flag 'abstract=on' which
indicates to use the abstract namespace. As an example:
qemu -chardev foo,path=bar,abstract=on
vs:
qemu -chardev foo,path=0bar
I think the former is quite a bit more user friendly.
OK
Regards,
Anthony Liguori
Regards,
Alex
---
qemu-sockets.c | 58 +---
1 file changed, 55 insertions(+), 3 deletions(-)
diff --git a/qemu-sockets.c b/qemu-sockets.c
index 3537bf3..70c8ad5 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -663,11 +663,50 @@ int inet_nonblocking_connect(const char *str,
#ifndef _WIN32
+/*
+ * The abstract socket namespace is a nonportable Linux extension.
+ * The sockets' names in this namespace have no connection
+ * with file system pathnames. To specify a named AF_UNIX socket
+ * in the abstract socket namespace, start the socket's path option
+ * with a backslash followed by zero: \0.
+ */
+static
+char *get_abstract_namespace_path(const char *path, int *abstract_path_length)
+{
+char *abstract_path;
+char *inp;
+char *outp;
+char c;
+
+*abstract_path_length = 0;
+if ((path == NULL) || (path[0] != '\\') || (path[1] != '0')) {
+return NULL;
+}
+abstract_path = g_strdup(path + 2);
+for (inp = abstract_path, outp = abstract_path; (c = *inp); ++inp, ++outp)
{
+if (c == '\\') {
+c = *++inp;
+if (c == '\\') {
+*outp = c;
+} else if (c == '0') {
+*outp = '\0';
+}
+} else {
+*outp = c;
+}
+}
+*abstract_path_length = outp - abstract_path;
+return abstract_path;
+}
+
int unix_listen_opts(QemuOpts *opts, Error **errp)
{
struct sockaddr_un un;
const char *path = qemu_opt_get(opts, path);
+int abstract_path_length;
+char *abstract_path;
int sock, fd;
+socklen_t addrlen = sizeof(un);
sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
if (sock 0) {
@@ -675,9 +714,18 @@ int unix_listen_opts(QemuOpts *opts, Error **errp)
return -1;
}
+abstract_path = get_abstract_namespace_path(path, abstract_path_length);
+
memset(un, 0, sizeof(un));
un.sun_family = AF_UNIX;
-if (path strlen(path)) {
+if (abstract_path != NULL) {
+if (abstract_path_length sizeof(un.sun_path) - 1) {
+abstract_path_length = sizeof(un.sun_path) - 1;
+}
+memcpy(un.sun_path + 1, abstract_path, abstract_path_length);
+addrlen =
+offsetof(struct sockaddr_un, sun_path) + 1 + abstract_path_length;
+} else if (path strlen(path)) {
snprintf(un.sun_path, sizeof(un.sun_path), %s, path);
} else {
char *tmpdir = getenv(TMPDIR);
@@ -694,8 +742,10 @@ int unix_listen_opts(QemuOpts *opts, Error **errp)
qemu_opt_set(opts, path, un.sun_path);
}
-unlink(un.sun_path);
-if (bind(sock, (struct sockaddr*) un, sizeof(un)) 0) {
+if (abstract_path == NULL) {
+unlink(un.sun_path);
+}
+if (bind(sock, (struct sockaddr *) un, addrlen) 0) {
error_set_errno(errp, errno, QERR_SOCKET_BIND_FAILED);
goto err;
}
@@ -704,10 +754,12 @@ int unix_listen_opts(QemuOpts *opts, Error **errp)
goto err;
}
+g_free(abstract_path);
return sock;
err:
closesocket(sock);
+g_free(abstract_path);
return -1;
}
--
1.7.9.5
[Qemu-devel] [PATCH 1/1] memory: add name in AddressSpace initialization.
Pass the AddressSpace's name (to be used for debugging)
to address_space_init(). If NULL is passed, the name
of root memory region is used instead.
Signed-off-by: Alexander Barabash alexander_barab...@mentor.com
---
exec.c|6 ++
hw/pci/pci.c |3 ++-
include/exec/memory.h |5 +++--
memory.c |5 +++--
4 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/exec.c b/exec.c
index a6923ad..4637c28 100644
--- a/exec.c
+++ b/exec.c
@@ -1784,13 +1784,11 @@ static void memory_map_init(void)
{
system_memory = g_malloc(sizeof(*system_memory));
memory_region_init(system_memory, system, INT64_MAX);
-address_space_init(address_space_memory, system_memory);
-address_space_memory.name = memory;
+address_space_init(address_space_memory, system_memory, memory);
system_io = g_malloc(sizeof(*system_io));
memory_region_init(system_io, io, 65536);
-address_space_init(address_space_io, system_io);
-address_space_io.name = I/O;
+address_space_init(address_space_io, system_io, I/O);
memory_listener_register(core_memory_listener, address_space_memory);
memory_listener_register(io_memory_listener, address_space_io);
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 94840c4..6cc3abe 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -790,7 +790,8 @@ static PCIDevice *do_pci_register_device(PCIDevice
*pci_dev, PCIBus *bus,
get_system_memory(), 0,
memory_region_size(get_system_memory()));
memory_region_set_enabled(pci_dev-bus_master_enable_region, false);
-address_space_init(pci_dev-bus_master_as,
pci_dev-bus_master_enable_region);
+address_space_init(pci_dev-bus_master_as,
+ pci_dev-bus_master_enable_region, NULL);
pci_dev-dma = g_new(DMAContext, 1);
dma_context_init(pci_dev-dma, pci_dev-bus_master_as, NULL, NULL,
NULL);
}
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 2322732..cb79a65 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -817,9 +817,10 @@ void mtree_info(fprintf_function mon_printf, void *f);
*
* @as: an uninitialized #AddressSpace
* @root: a #MemoryRegion that routes addesses for the address space
+ * @name: used for debugging. If NULL, name of the root memory region is used.
*/
-void address_space_init(AddressSpace *as, MemoryRegion *root);
-
+void address_space_init(AddressSpace *as, MemoryRegion *root,
+ const char *name);
/**
* address_space_destroy: destroy an address space
diff --git a/memory.c b/memory.c
index 410c5f8..ab3b136 100644
--- a/memory.c
+++ b/memory.c
@@ -1562,14 +1562,14 @@ void memory_listener_unregister(MemoryListener
*listener)
QTAILQ_REMOVE(memory_listeners, listener, link);
}
-void address_space_init(AddressSpace *as, MemoryRegion *root)
+void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
{
memory_region_transaction_begin();
as-root = root;
as-current_map = g_new(FlatView, 1);
flatview_init(as-current_map);
QTAILQ_INSERT_TAIL(address_spaces, as, address_spaces_link);
-as-name = NULL;
+as-name = g_strdup(name ? name : root-name);
memory_region_transaction_commit();
address_space_init_dispatch(as);
}
@@ -1584,6 +1584,7 @@ void address_space_destroy(AddressSpace *as)
address_space_destroy_dispatch(as);
flatview_destroy(as-current_map);
g_free(as-current_map);
+g_free((char *)as-name);
}
uint64_t io_mem_read(MemoryRegion *mr, hwaddr addr, unsigned size)
--
1.7.9.5
Re: [Qemu-devel] [PATCH 1/1] memory: add name in AddressSpace initialization.
On 01/08/2013 04:00 PM, Anthony Liguori wrote:
Alexander Barabash alexander_barab...@mentor.com writes:
Pass the AddressSpace's name (to be used for debugging)
to address_space_init(). If NULL is passed, the name
of root memory region is used instead.
@@ -1784,13 +1784,11 @@ static void memory_map_init(void)
...
memory_region_init(system_io, io, 65536);
-address_space_init(address_space_io, system_io);
-address_space_io.name = I/O;
+address_space_init(address_space_io, system_io, I/O);
It's best to avoid special characters in names. It will make a
conversion to QOM later a lot easier. So s:I/O:IO:g'
I did not change the name of this address space.
We can do it in a separate patch.
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -790,7 +790,8 @@ static PCIDevice *do_pci_register_device(PCIDevice
*pci_dev, PCIBus *bus,
get_system_memory(), 0,
memory_region_size(get_system_memory()));
memory_region_set_enabled(pci_dev-bus_master_enable_region, false);
-address_space_init(pci_dev-bus_master_as,
pci_dev-bus_master_enable_region);
+address_space_init(pci_dev-bus_master_as,
+ pci_dev-bus_master_enable_region, NULL);
Please top-post new patches.
I assume you this to mean, post new patches as reply to the previous
one. ) OK
Any reason to not name this address space?
It was not named previously. Now, it will pick up the name from the root
memory region,
passed to the same call.
Regards,
Anthony Liguori
Regards,
Alex Barabash
pci_dev-dma = g_new(DMAContext, 1);
dma_context_init(pci_dev-dma, pci_dev-bus_master_as, NULL, NULL,
NULL);
}
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 2322732..cb79a65 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -817,9 +817,10 @@ void mtree_info(fprintf_function mon_printf, void *f);
*
* @as: an uninitialized #AddressSpace
* @root: a #MemoryRegion that routes addesses for the address space
+ * @name: used for debugging. If NULL, name of the root memory region is used.
*/
-void address_space_init(AddressSpace *as, MemoryRegion *root);
-
+void address_space_init(AddressSpace *as, MemoryRegion *root,
+ const char *name);
/**
* address_space_destroy: destroy an address space
diff --git a/memory.c b/memory.c
index 410c5f8..ab3b136 100644
--- a/memory.c
+++ b/memory.c
@@ -1562,14 +1562,14 @@ void memory_listener_unregister(MemoryListener
*listener)
QTAILQ_REMOVE(memory_listeners, listener, link);
}
-void address_space_init(AddressSpace *as, MemoryRegion *root)
+void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
{
memory_region_transaction_begin();
as-root = root;
as-current_map = g_new(FlatView, 1);
flatview_init(as-current_map);
QTAILQ_INSERT_TAIL(address_spaces, as, address_spaces_link);
-as-name = NULL;
+as-name = g_strdup(name ? name : root-name);
memory_region_transaction_commit();
address_space_init_dispatch(as);
}
@@ -1584,6 +1584,7 @@ void address_space_destroy(AddressSpace *as)
address_space_destroy_dispatch(as);
flatview_destroy(as-current_map);
g_free(as-current_map);
+g_free((char *)as-name);
}
uint64_t io_mem_read(MemoryRegion *mr, hwaddr addr, unsigned size)
--
1.7.9.5
[Qemu-devel] [PATCH v2 1/1] chardev: Support abstract socket namespace in AF_UNIX socket family.
The abstract socket namespace is a nonportable Linux extension.
The sockets' names in this namespace have no connection
with file system pathnames. To specify a named AF_UNIX socket
in the abstract socket namespace, pass true for the boolean flag
abstract, e.g.:
qemu -chardev socket,path=NAME_OF_THE_SOCKET,abstract=on
Signed-off-by: Alexander Barabash alexander_barab...@mentor.com
---
qemu-config.c |3 +++
qemu-sockets.c | 35 ++-
2 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/qemu-config.c b/qemu-config.c
index 2188c3e..5b0f71e 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -182,6 +182,9 @@ static QemuOptsList qemu_chardev_opts = {
.name = ipv6,
.type = QEMU_OPT_BOOL,
},{
+.name = abstract,
+.type = QEMU_OPT_BOOL,
+},{
.name = wait,
.type = QEMU_OPT_BOOL,
},{
diff --git a/qemu-sockets.c b/qemu-sockets.c
index 3537bf3..b1c3a79 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -56,6 +56,9 @@ static QemuOptsList dummy_opts = {
},{
.name = ipv6,
.type = QEMU_OPT_BOOL,
+},{
+.name = abstract,
+.type = QEMU_OPT_BOOL,
},
{ /* end if list */ }
},
@@ -667,7 +670,9 @@ int unix_listen_opts(QemuOpts *opts, Error **errp)
{
struct sockaddr_un un;
const char *path = qemu_opt_get(opts, path);
+bool abstract = qemu_opt_get_bool(opts, abstract, false);
int sock, fd;
+socklen_t addrlen = sizeof(un);
sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
if (sock 0) {
@@ -678,7 +683,15 @@ int unix_listen_opts(QemuOpts *opts, Error **errp)
memset(un, 0, sizeof(un));
un.sun_family = AF_UNIX;
if (path strlen(path)) {
-snprintf(un.sun_path, sizeof(un.sun_path), %s, path);
+if (!abstract) {
+snprintf(un.sun_path, sizeof(un.sun_path), %s, path);
+} else {
+int printed_chars;
+printed_chars = snprintf(un.sun_path + 1, sizeof(un.sun_path) - 1,
+ %s, path);
+addrlen =
+offsetof(struct sockaddr_un, sun_path) + 1 + printed_chars;
+}
} else {
char *tmpdir = getenv(TMPDIR);
snprintf(un.sun_path, sizeof(un.sun_path), %s/qemu-socket-XX,
@@ -694,8 +707,10 @@ int unix_listen_opts(QemuOpts *opts, Error **errp)
qemu_opt_set(opts, path, un.sun_path);
}
-unlink(un.sun_path);
-if (bind(sock, (struct sockaddr*) un, sizeof(un)) 0) {
+if (!abstract) {
+unlink(un.sun_path);
+}
+if (bind(sock, (struct sockaddr *) un, addrlen) 0) {
error_set_errno(errp, errno, QERR_SOCKET_BIND_FAILED);
goto err;
}
@@ -716,8 +731,10 @@ int unix_connect_opts(QemuOpts *opts, Error **errp,
{
struct sockaddr_un un;
const char *path = qemu_opt_get(opts, path);
+bool abstract = qemu_opt_get_bool(opts, abstract, false);
ConnectState *connect_state = NULL;
int sock, rc;
+socklen_t addrlen = sizeof(un);
if (NULL == path) {
error_setg(errp, unix connect: no path specified\n);
@@ -738,12 +755,20 @@ int unix_connect_opts(QemuOpts *opts, Error **errp,
memset(un, 0, sizeof(un));
un.sun_family = AF_UNIX;
-snprintf(un.sun_path, sizeof(un.sun_path), %s, path);
+if (!abstract) {
+snprintf(un.sun_path, sizeof(un.sun_path), %s, path);
+} else {
+int printed_chars;
+printed_chars = snprintf(un.sun_path + 1, sizeof(un.sun_path) - 1,
+ %s, path);
+addrlen =
+offsetof(struct sockaddr_un, sun_path) + 1 + printed_chars;
+}
/* connect to peer */
do {
rc = 0;
-if (connect(sock, (struct sockaddr *) un, sizeof(un)) 0) {
+if (connect(sock, (struct sockaddr *) un, addrlen) 0) {
rc = -socket_error();
}
} while (rc == -EINTR);
--
1.7.9.5
[Qemu-devel] [PATCH v3 1/1] memory: add name in AddressSpace initialization.
Pass the AddressSpace's name (to be used for debugging)
to address_space_init(). If NULL is passed, the name
of root memory region is used instead.
Signed-off-by: Alexander Barabash alexander_barab...@mentor.com
---
exec.c|6 ++
hw/pci/pci.c |3 ++-
include/exec/memory.h |5 +++--
memory.c |5 +++--
4 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/exec.c b/exec.c
index a6923ad..b22abcc 100644
--- a/exec.c
+++ b/exec.c
@@ -1784,13 +1784,11 @@ static void memory_map_init(void)
{
system_memory = g_malloc(sizeof(*system_memory));
memory_region_init(system_memory, system, INT64_MAX);
-address_space_init(address_space_memory, system_memory);
-address_space_memory.name = memory;
+address_space_init(address_space_memory, system_memory, memory);
system_io = g_malloc(sizeof(*system_io));
memory_region_init(system_io, io, 65536);
-address_space_init(address_space_io, system_io);
-address_space_io.name = I/O;
+address_space_init(address_space_io, system_io, IO);
memory_listener_register(core_memory_listener, address_space_memory);
memory_listener_register(io_memory_listener, address_space_io);
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 94840c4..6cc3abe 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -790,7 +790,8 @@ static PCIDevice *do_pci_register_device(PCIDevice
*pci_dev, PCIBus *bus,
get_system_memory(), 0,
memory_region_size(get_system_memory()));
memory_region_set_enabled(pci_dev-bus_master_enable_region, false);
-address_space_init(pci_dev-bus_master_as,
pci_dev-bus_master_enable_region);
+address_space_init(pci_dev-bus_master_as,
+ pci_dev-bus_master_enable_region, NULL);
pci_dev-dma = g_new(DMAContext, 1);
dma_context_init(pci_dev-dma, pci_dev-bus_master_as, NULL, NULL,
NULL);
}
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 2322732..cb79a65 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -817,9 +817,10 @@ void mtree_info(fprintf_function mon_printf, void *f);
*
* @as: an uninitialized #AddressSpace
* @root: a #MemoryRegion that routes addesses for the address space
+ * @name: used for debugging. If NULL, name of the root memory region is used.
*/
-void address_space_init(AddressSpace *as, MemoryRegion *root);
-
+void address_space_init(AddressSpace *as, MemoryRegion *root,
+ const char *name);
/**
* address_space_destroy: destroy an address space
diff --git a/memory.c b/memory.c
index 410c5f8..ab3b136 100644
--- a/memory.c
+++ b/memory.c
@@ -1562,14 +1562,14 @@ void memory_listener_unregister(MemoryListener
*listener)
QTAILQ_REMOVE(memory_listeners, listener, link);
}
-void address_space_init(AddressSpace *as, MemoryRegion *root)
+void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
{
memory_region_transaction_begin();
as-root = root;
as-current_map = g_new(FlatView, 1);
flatview_init(as-current_map);
QTAILQ_INSERT_TAIL(address_spaces, as, address_spaces_link);
-as-name = NULL;
+as-name = g_strdup(name ? name : root-name);
memory_region_transaction_commit();
address_space_init_dispatch(as);
}
@@ -1584,6 +1584,7 @@ void address_space_destroy(AddressSpace *as)
address_space_destroy_dispatch(as);
flatview_destroy(as-current_map);
g_free(as-current_map);
+g_free((char *)as-name);
}
uint64_t io_mem_read(MemoryRegion *mr, hwaddr addr, unsigned size)
--
1.7.9.5
[Qemu-devel] [PATCH 1/1] Added address_space_init2().
address_space_init2: initializes a named address space.
Signed-off-by: Alexander Barabash alexander_barab...@mentor.com
---
include/exec/memory.h |9 +
memory.c |6 ++
2 files changed, 15 insertions(+)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 2322732..8f8a31d 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -820,6 +820,15 @@ void mtree_info(fprintf_function mon_printf, void *f);
*/
void address_space_init(AddressSpace *as, MemoryRegion *root);
+/**
+ * address_space_init2: initializes a named address space
+ *
+ * @as: an uninitialized #AddressSpace
+ * @root: a #MemoryRegion that routes addesses for the address space
+ * @name: used for debugging
+ */
+void address_space_init2(AddressSpace *as, MemoryRegion *root,
+ const char *name);
/**
* address_space_destroy: destroy an address space
diff --git a/memory.c b/memory.c
index 410c5f8..1652c10 100644
--- a/memory.c
+++ b/memory.c
@@ -1574,6 +1574,12 @@ void address_space_init(AddressSpace *as, MemoryRegion
*root)
address_space_init_dispatch(as);
}
+void address_space_init2(AddressSpace *as, MemoryRegion *root, const char
*name)
+{
+address_space_init(as, root);
+as-name = g_strdup(name);
+}
+
void address_space_destroy(AddressSpace *as)
{
/* Flush out anything from MemoryListeners listening in on this */
--
1.7.9.5
[Qemu-devel] [PATCH 1/1] Support 2**64 bytes memory regions.
Memory regions created with size UINT64_MAX
are currently treated as regions of size 2**64 bytes.
This patch adds full support for such regions.
Signed-off-by: Alexander Barabash alexander_barab...@mentor.com
---
exec.c |4
memory.c | 72 --
2 files changed, 55 insertions(+), 21 deletions(-)
diff --git a/exec.c b/exec.c
index 140eb56..8ce8ab4 100644
--- a/exec.c
+++ b/exec.c
@@ -789,6 +789,10 @@ static void mem_add(MemoryListener *listener,
MemoryRegionSection *section)
remain.size -= now.size;
remain.offset_within_address_space += now.size;
remain.offset_within_region += now.size;
+if ((remain.size == TARGET_PAGE_SIZE - 1)
+(section-size == UINT64_MAX)) {
+remain.size = TARGET_PAGE_SIZE;
+}
}
now = remain;
if (now.size) {
diff --git a/memory.c b/memory.c
index 1652c10..b30edc8 100644
--- a/memory.c
+++ b/memory.c
@@ -84,6 +84,19 @@ static AddrRange addrrange_intersection(AddrRange r1,
AddrRange r2)
return addrrange_make(start, int128_sub(end, start));
}
+static uint64_t mr_size_get64(Int128 size)
+{
+if (int128_eq(size, int128_2_64())) {
+return UINT64_MAX;
+}
+return int128_get64(size);
+}
+
+static uint64_t addrrange_size_get64(const AddrRange *r)
+{
+return mr_size_get64(r-size);
+}
+
enum ListenerDirection { Forward, Reverse };
static bool memory_listener_match(MemoryListener *listener,
@@ -93,6 +106,18 @@ static bool memory_listener_match(MemoryListener *listener,
|| listener-address_space_filter == section-address_space;
}
+static hwaddr memory_region_get_last_addr(const MemoryRegion *mr, hwaddr base)
+{
+return int128_get64(int128_sub(int128_add(int128_make64(base + mr-addr),
+ mr-size), int128_one()));
+}
+
+static hwaddr memory_region_get_last_offset(const MemoryRegion *mr)
+{
+return int128_get64(int128_sub(int128_add(int128_make64(mr-alias_offset),
+ mr-size), int128_one()));
+}
+
#define MEMORY_LISTENER_CALL_GLOBAL(_callback, _direction, _args...)\
do {\
MemoryListener *_listener; \
@@ -150,7 +175,7 @@ static bool memory_listener_match(MemoryListener *listener,
.mr = (fr)-mr, \
.address_space = (as), \
.offset_within_region = (fr)-offset_in_region, \
-.size = int128_get64((fr)-addr.size), \
+.size = flatrange_size_get64(fr), \
.offset_within_address_space = int128_get64((fr)-addr.start), \
.readonly = (fr)-readonly, \
}))
@@ -204,6 +229,12 @@ static bool
memory_region_ioeventfd_equal(MemoryRegionIoeventfd a,
!memory_region_ioeventfd_before(b, a);
}
+static
+uint64_t memory_region_ioeventfd_size_get64(const MemoryRegionIoeventfd *fd)
+{
+return addrrange_size_get64(fd-addr);
+}
+
typedef struct FlatRange FlatRange;
typedef struct FlatView FlatView;
@@ -240,6 +271,11 @@ static bool flatrange_equal(FlatRange *a, FlatRange *b)
a-readonly == b-readonly;
}
+static uint64_t flatrange_size_get64(const FlatRange *fr)
+{
+return addrrange_size_get64(fr-addr);
+}
+
static void flatview_init(FlatView *view)
{
view-ranges = NULL;
@@ -598,7 +634,7 @@ static void address_space_add_del_ioeventfds(AddressSpace
*as,
section = (MemoryRegionSection) {
.address_space = as,
.offset_within_address_space = int128_get64(fd-addr.start),
-.size = int128_get64(fd-addr.size),
+.size = memory_region_ioeventfd_size_get64(fd),
};
MEMORY_LISTENER_CALL(eventfd_del, Forward, section,
fd-match_data, fd-data, fd-e);
@@ -611,7 +647,7 @@ static void address_space_add_del_ioeventfds(AddressSpace
*as,
section = (MemoryRegionSection) {
.address_space = as,
.offset_within_address_space = int128_get64(fd-addr.start),
-.size = int128_get64(fd-addr.size),
+.size = memory_region_ioeventfd_size_get64(fd),
};
MEMORY_LISTENER_CALL(eventfd_add, Reverse, section,
fd-match_data, fd-data, fd-e);
@@ -1030,10 +1066,7 @@ void memory_region_destroy(MemoryRegion *mr)
uint64_t memory_region_size(MemoryRegion *mr)
{
-if (int128_eq(mr-size, int128_2_64())) {
-return UINT64_MAX;
-}
-return int128_get64(mr-size);
+return mr_size_get64(mr-size);
}
const char *memory_region_name(MemoryRegion *mr)
@@ -1163,12 +1196,12
[Qemu-devel] [PATCH 1/1] Support abstract socket namespace in AF_UNIX socket family.
The abstract socket namespace is a nonportable Linux extension.
The sockets' names in this namespace have no connection
with file system pathnames. To specify a named AF_UNIX socket
in the abstract socket namespace, start the socket's path option
with a backslash followed by zero: \0.
Signed-off-by: Alexander Barabash alexander_barab...@mentor.com
---
qemu-sockets.c | 58 +---
1 file changed, 55 insertions(+), 3 deletions(-)
diff --git a/qemu-sockets.c b/qemu-sockets.c
index 3537bf3..70c8ad5 100644
--- a/qemu-sockets.c
+++ b/qemu-sockets.c
@@ -663,11 +663,50 @@ int inet_nonblocking_connect(const char *str,
#ifndef _WIN32
+/*
+ * The abstract socket namespace is a nonportable Linux extension.
+ * The sockets' names in this namespace have no connection
+ * with file system pathnames. To specify a named AF_UNIX socket
+ * in the abstract socket namespace, start the socket's path option
+ * with a backslash followed by zero: \0.
+ */
+static
+char *get_abstract_namespace_path(const char *path, int *abstract_path_length)
+{
+char *abstract_path;
+char *inp;
+char *outp;
+char c;
+
+*abstract_path_length = 0;
+if ((path == NULL) || (path[0] != '\\') || (path[1] != '0')) {
+return NULL;
+}
+abstract_path = g_strdup(path + 2);
+for (inp = abstract_path, outp = abstract_path; (c = *inp); ++inp, ++outp)
{
+if (c == '\\') {
+c = *++inp;
+if (c == '\\') {
+*outp = c;
+} else if (c == '0') {
+*outp = '\0';
+}
+} else {
+*outp = c;
+}
+}
+*abstract_path_length = outp - abstract_path;
+return abstract_path;
+}
+
int unix_listen_opts(QemuOpts *opts, Error **errp)
{
struct sockaddr_un un;
const char *path = qemu_opt_get(opts, path);
+int abstract_path_length;
+char *abstract_path;
int sock, fd;
+socklen_t addrlen = sizeof(un);
sock = qemu_socket(PF_UNIX, SOCK_STREAM, 0);
if (sock 0) {
@@ -675,9 +714,18 @@ int unix_listen_opts(QemuOpts *opts, Error **errp)
return -1;
}
+abstract_path = get_abstract_namespace_path(path, abstract_path_length);
+
memset(un, 0, sizeof(un));
un.sun_family = AF_UNIX;
-if (path strlen(path)) {
+if (abstract_path != NULL) {
+if (abstract_path_length sizeof(un.sun_path) - 1) {
+abstract_path_length = sizeof(un.sun_path) - 1;
+}
+memcpy(un.sun_path + 1, abstract_path, abstract_path_length);
+addrlen =
+offsetof(struct sockaddr_un, sun_path) + 1 + abstract_path_length;
+} else if (path strlen(path)) {
snprintf(un.sun_path, sizeof(un.sun_path), %s, path);
} else {
char *tmpdir = getenv(TMPDIR);
@@ -694,8 +742,10 @@ int unix_listen_opts(QemuOpts *opts, Error **errp)
qemu_opt_set(opts, path, un.sun_path);
}
-unlink(un.sun_path);
-if (bind(sock, (struct sockaddr*) un, sizeof(un)) 0) {
+if (abstract_path == NULL) {
+unlink(un.sun_path);
+}
+if (bind(sock, (struct sockaddr *) un, addrlen) 0) {
error_set_errno(errp, errno, QERR_SOCKET_BIND_FAILED);
goto err;
}
@@ -704,10 +754,12 @@ int unix_listen_opts(QemuOpts *opts, Error **errp)
goto err;
}
+g_free(abstract_path);
return sock;
err:
closesocket(sock);
+g_free(abstract_path);
return -1;
}
--
1.7.9.5
[Qemu-devel] [PATCH 1/1] Support gdbstub qXfer:spaces features.
Support qXfer:spaces:read and qXfer:spaces:write.
These gdbstub features allow GDB to access target's
hardware registers.
Signed-off-by: Alexander Barabash alexander_barab...@mentor.com
---
Makefile.target |1 +
gdbstub.c | 176 ++-
xfer-spaces.c | 419 +++
xfer-spaces.h | 74 ++
4 files changed, 667 insertions(+), 3 deletions(-)
create mode 100644 xfer-spaces.c
create mode 100644 xfer-spaces.h
diff --git a/Makefile.target b/Makefile.target
index 5bfa4960..361ef2b 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -109,6 +109,7 @@ CONFIG_NO_GET_MEMORY_MAPPING = $(if $(subst
n,,$(CONFIG_HAVE_GET_MEMORY_MAPPING)
CONFIG_NO_CORE_DUMP = $(if $(subst n,,$(CONFIG_HAVE_CORE_DUMP)),n,y)
obj-y += arch_init.o cpus.o monitor.o gdbstub.o balloon.o ioport.o
+obj-y += xfer-spaces.o
obj-y += hw/
obj-$(CONFIG_KVM) += kvm-all.o
obj-$(CONFIG_NO_KVM) += kvm-stub.o
diff --git a/gdbstub.c b/gdbstub.c
index a8dd437..f1c2208 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -18,6 +18,7 @@
*/
#include config.h
#include qemu-common.h
+#include xfer-spaces.h
#ifdef CONFIG_USER_ONLY
#include stdlib.h
#include stdio.h
@@ -1798,6 +1799,35 @@ static int memtox(char *buf, const char *mem, int len)
return p - buf;
}
+/* Decode data using the encoding for 'x' packets. */
+static int xtomem(uint8_t *mem, const char *buf, int len)
+{
+uint8_t *p = mem;
+char c;
+bool escaped = false;
+
+while (len--) {
+c = *(buf++);
+if (escaped) {
+escaped = false;
+*(p++) = c ^ 0x20;
+} else {
+if (c == '}') {
+escaped = true;
+} else {
+*(p++) = c;
+}
+}
+}
+
+if (escaped) {
+fprintf(stderr, Unmatched escape character in target response.\n);
+return 0;
+}
+
+return p - mem;
+}
+
static const char *get_feature_xml(const char *p, const char **newp)
{
size_t len;
@@ -1832,6 +1862,9 @@ static const char *get_feature_xml(const char *p, const
char **newp)
}
return target_xml;
}
+if (strncmp(p, xfer-spaces.xml, len) == 0) {
+return xfer_spaces_get_xml();
+}
for (i = 0; ; i++) {
name = xml_builtin[i][0];
if (!name || (strncmp(name, p, len) == 0 strlen(name) == len))
@@ -2058,7 +2091,8 @@ static CPUArchState *find_cpu(uint32_t thread_id)
return NULL;
}
-static int gdb_handle_packet(GDBState *s, const char *line_buf)
+static int gdb_handle_packet(GDBState *s, const char *line_buf,
+ const char *line_buf_end)
{
CPUArchState *env;
const char *p;
@@ -2424,7 +2458,9 @@ static int gdb_handle_packet(GDBState *s, const char
*line_buf)
if (strncmp(p, Supported, 9) == 0) {
snprintf(buf, sizeof(buf), PacketSize=%x, MAX_PACKET_LENGTH);
#ifdef GDB_CORE_XML
-pstrcat(buf, sizeof(buf), ;qXfer:features:read+);
+pstrcat(buf, sizeof(buf),
+;qXfer:features:read+
+;qXfer:spaces:read+;qXfer:spaces:write+);
#endif
put_packet(s, buf);
break;
@@ -2468,6 +2504,139 @@ static int gdb_handle_packet(GDBState *s, const char
*line_buf)
put_packet_binary(s, buf, len + 1);
break;
}
+
+if (strncmp(p, Xfer:spaces:read:, strlen(Xfer:spaces:read:)) == 0)
{
+bool request_wellformed = false;
+const char *space_name;
+uint64_t offset;
+uint64_t length;
+char **colon_tokens;
+const char *offset_and_length;
+char **offset_and_length_tokens = NULL;
+char *offset_string;
+char *length_string;
+
+p += strlen(Xfer:spaces:read:);
+colon_tokens = g_strsplit(p, :, 2);
+do {
+space_name = colon_tokens[0];
+if (space_name == NULL) {
+break;
+}
+offset_and_length = colon_tokens[1];
+if (offset_and_length == NULL) {
+break;
+}
+offset_and_length_tokens = g_strsplit(offset_and_length,
+ ,, 2);
+offset_string = offset_and_length_tokens[0];
+if (offset_string == NULL) {
+break;
+}
+length_string = offset_and_length_tokens[1];
+if (length_string == NULL) {
+break;
+}
+
+offset = g_ascii_strtoull(offset_string, offset_string, 16);
+if (offset G_MAXUINT32) {
+break;
+}
+if (*offset_string != '\0') {
+break;
+}
+
+length = g_ascii_strtoull
Re: [Qemu-devel] [PATCH] qom: Make object_unref() free the object's memory when refcount goes to 0.
On 02/24/2012 05:11 PM, Anthony Liguori wrote:
On 02/23/2012 10:21 AM, Alexander Barabash wrote:
On 02/22/2012 09:12 PM, Anthony Liguori wrote:
On 02/22/2012 12:00 PM, alexander_barab...@mentor.com wrote:
From: Alexander Barabashalexander_barab...@mentor.com
Why do you want to have a delete notifier list, rather than just a
delete callback.
Because a notifier list allows for third parties to receive the event
(think GObject signal/slots).
This is a valid point, but wouldn't it logical to issue an event before
running the destructor?
Along the lines:
void object_finalize(void *data)
{
Object *obj = data;
TypeImpl *ti = obj-class-type;
object_deinit(obj, ti);
object_property_del_all(obj);
g_assert(obj-ref == 0);
object_finalized_notification(obj);
}
...
void object_unref(Object *obj)
{
g_assert(obj-ref 0);
if (obj-ref == 1) {
object_is_about_to_be_finalized_notification(obj);
}
obj-ref--;
/* parent always holds a reference to its children */
if (obj-ref == 0) {
object_finalize(obj);
}
}
Here, there is a notification while the object is still alive (in the
sense that it has not been finalized).
Then, if the object is actually finalized, there is notification about that.
By the way, using weak references would spare us the notification list.
Object's memory will not be freed as long as a weak reference to it exists.
Access through a weak reference to a dead object will remove that weak
reference.
This way, we shall also avoid problems with circular references between
objects.
Regards,
Alex
At the point where refcount == 0, the destructor has been called
already,
so there is not much to be done, except for reclaim the memory.
Right, but the memory is not allocated by the core of Object. This is
important in order to allow in-place object creation. You could
special case this and have a flag to indicate whether the object has
allocated it's own memory or not but I think the two approaches end up
having equal complexity whereas the NotifierList gives you a lot more
flexibility.
It makes it possible to use a small object allocator for Objects which
could be useful one day if we use objects in a fast path (like using
Objects to allocate packets in the network layer or requests in the
block layer).
Regards,
Anthony Liguori
Re: [Qemu-devel] [PATCH] qom: Make object_unref() free the object's memory when refcount goes to 0.
On 02/22/2012 09:12 PM, Anthony Liguori wrote:
On 02/22/2012 12:00 PM, alexander_barab...@mentor.com wrote:
From: Alexander Barabashalexander_barab...@mentor.com
In the existing implementation, object_delete()
calls object_unref(), then frees the object's storage.
Running object_delete() on an object with reference count
different from 1 causes program failure.
In the existing implementation, object_unref()
finalizes the object when its reference count becomes 0.
In the new implementation, object_unref()
finalizes and frees the object's storage when the reference count
becomes 0.
In the new implementation, object_delete()
just calls object_unref().
Running object_delete() on an object with reference count
different from 1 still causes program failure.
This isn't correct. QOM objects don't necessarily have heap allocated
objects.
I've been thinking about this general problem and I think the right
way to solve it is to have a delete notifier list. That way,
object_new() can register a delete notifier that calls g_free()
whenever refcount=0. That way an explicit object_delete() isn't
needed anymore.
Why do you want to have a delete notifier list, rather than just a
delete callback.
At the point where refcount == 0, the destructor has been called already,
so there is not much to be done, except for reclaim the memory.
Regards,
Alex
Although I think we should keep the call around as it's convenient for
replacing occurrences of qdev_free() where you really want the assert.
Regards,
Anthony Liguori
Signed-off-by: Alexander Barabashalexander_barab...@mentor.com
---
qom/object.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/qom/object.c b/qom/object.c
index e6591e1..8d36a9c 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -373,9 +373,8 @@ Object *object_new(const char *typename)
void object_delete(Object *obj)
{
+g_assert(obj-ref == 1);
object_unref(obj);
-g_assert(obj-ref == 0);
-g_free(obj);
}
static bool type_is_ancestor(TypeImpl *type, TypeImpl *target_type)
@@ -585,6 +584,7 @@ void object_unref(Object *obj)
/* parent always holds a reference to its children */
if (obj-ref == 0) {
object_finalize(obj);
+g_free(obj);
}
}
Re: [Qemu-devel] [PATCH] qom: In function object_set_link_property(), first call object_ref(), then object_unref().
On 02/22/2012 07:17 PM, Paolo Bonzini wrote:
On 02/22/2012 06:13 PM, alexander_barab...@mentor.com wrote:
From: Alexander Barabashalexander_barab...@mentor.com
In the old implementation, if the new value of the property links
to the same object, as the old value, that object is first unref-ed,
and then ref-ed. This leads to unintended deinitialization of that object.
In the new implementation, this is fixed.
Signed-off-by: Alexander Barabashalexander_barab...@mentor.com
---
qom/object.c | 18 +-
1 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/qom/object.c b/qom/object.c
index 941c291..d1b3ac7 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -892,19 +892,19 @@ static void object_set_link_property(Object *obj, Visitor
*v, void *opaque,
const char *name, Error **errp)
{
Object **child = opaque;
+Object *old_target;
bool ambiguous = false;
const char *type;
char *path;
gchar *target_type;
+bool clear_old_target = true;
type = object_property_get_type(obj, name, NULL);
visit_type_str(v,path, name, errp);
-if (*child) {
-object_unref(*child);
-*child = NULL;
-}
+old_target = *child;
+*child = NULL;
You can just remove the unref here...
if (strcmp(path, ) != 0) {
Object *target;
@@ -916,7 +916,11 @@ static void object_set_link_property(Object *obj, Visitor
*v, void *opaque,
if (ambiguous) {
error_set(errp, QERR_AMBIGUOUS_PATH, path);
} else if (target) {
-object_ref(target);
+if (target != old_target) {
+object_ref(target);
... leave the unconditional ref to target here...
+} else {
+clear_old_target = false;
+}
*child = target;
} else {
target = object_resolve_path(path,ambiguous);
@@ -930,6 +934,10 @@ static void object_set_link_property(Object *obj, Visitor
*v, void *opaque,
}
g_free(path);
+
+if (clear_old_target (old_target != NULL)) {
+object_unref(old_target);
... and leave this unref on old_target, without the need for
clear_old_target.
+}
}
void object_property_add_link(Object *obj, const char *name,
Paolo
Agreed.
Alex
Re: [Qemu-devel] [PATCH] qom: In function object_set_link_property(), first call object_ref(), then object_unref().
On 02/22/2012 07:24 PM, Paolo Bonzini wrote:
On 02/22/2012 06:22 PM, alexander_barab...@mentor.com wrote:
From: Alexander Barabashalexander_barab...@mentor.com
In the old implementation, if the new value of the property links
to the same object, as the old value, that object is first unref-ed,
and then ref-ed. This leads to unintended deinitialization of that object.
In the new implementation, this is fixed.
Signed-off-by: Alexander Barabashalexander_barab...@mentor.com
---
qom/object.c | 11 +++
1 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/qom/object.c b/qom/object.c
index 941c291..e6591e1 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -892,6 +892,7 @@ static void object_set_link_property(Object *obj, Visitor
*v, void *opaque,
const char *name, Error **errp)
{
Object **child = opaque;
+Object *old_target;
bool ambiguous = false;
const char *type;
char *path;
@@ -901,10 +902,8 @@ static void object_set_link_property(Object *obj, Visitor
*v, void *opaque,
visit_type_str(v,path, name, errp);
-if (*child) {
-object_unref(*child);
-*child = NULL;
-}
+old_target = *child;
+*child = NULL;
if (strcmp(path, ) != 0) {
Object *target;
@@ -930,6 +929,10 @@ static void object_set_link_property(Object *obj, Visitor
*v, void *opaque,
}
g_free(path);
+
+if (old_target != NULL) {
+object_unref(old_target);
+}
}
void object_property_add_link(Object *obj, const char *name,
Reviewed-by: Paolo Bonzinipbonz...@redhat.com
Sorry, I did not figure out yet how to add the v2 to the subject line. ))
Alex
Re: [Qemu-devel] [PATCH] Add object_property_get_child().
On 02/20/2012 11:11 AM, Paolo Bonzini wrote: On 02/19/2012 01:36 PM, Alexander Barabash wrote: The proposed object_property_get_child() may return either the direct child with the specified name in the composition tree, or the value of the link with the specified name, as object_property_get_link() indeed does. Have you actually checked that object_property_get_link() doesn't work for you? It seems to me that your patch does the same as the one-liner: #define object_property_get_child object_property_get_link Now _this_ would be good for qemu-trivial. ;) You are right. Please disregard my v2 patch. I would only suggest that the documentation of object_property_add_child() be amended to reflect the fact that the child object may be retrieved using object_property_get_link(). but does it in a more straightforward way. I disagree that it is more straightforward. It is more *direct* perhaps, in that it doesn't go through visitors, object_get_child_property and object_resolve_path, but it also violates the encapsulation that properties provide quite heavily. Peeking at the opaque values of properties should be done as sparingly as possible. Ideally once, since no QOM property code should be in a fast path. In this case, it is done in object_resolve_abs_path, and that should be enough. Paolo Alex
Re: [Qemu-devel] [PATCH v2] Revised: Add object_property_get_child().
On 02/19/2012 06:04 PM, Alexander Barabash wrote: Add object_property_get_child(). Please disregard this patch. object_property_get_link() works for this purpose.
Re: [Qemu-devel] [PATCH v2] Revised: Add object_property_get_child().
On 02/19/2012 07:14 PM, Andreas Färber wrote:
Am 19.02.2012 17:04, schrieb Alexander Barabash:
...
Signed-off-by: Alexander Barabashalexander_barab...@mentor.com
Please use git-send-email to submit your patches: The commit message is
unnecessarily indented and the first line is duplicated. Instead of
Revised: (which v2 already indicates) the subject should mention the
topic, here qom: .
http://wiki.qemu.org/Contribute/SubmitAPatch
Thanks for the advice.
Your patch is doing too many things at once. Please split it up into a
series of easily digestable and bisectable patches, e.g.:
* ..._PREFIX/SUFFIX introduction and use in _add_child/link,
* ..._is_child/_is_link introduction and use in
_property_del_child/_get_canonical_path/_resolve_partial_path,
* link_type_to_type() and use in _set_link_property,
* REPORT_OBJECT_ERROR(),
* object_property_get_child().
OK
+/* Go from LINK_PROPERTY_TYPE_PREFIX FOO LINK_PROPERTY_TYPE_SUFFIX to
FOO. */
+static gchar *link_type_to_type(const gchar *type)
+{
+return g_strndup(type[sizeof(LINK_PROPERTY_TYPE_PREFIX) - 1],
+ strlen(type)
+ - (sizeof(LINK_PROPERTY_TYPE_PREFIX) - 1)
+ - (sizeof(LINK_PROPERTY_TYPE_SUFFIX) - 1));
Any reason not to use strlen()? I don't think this is a hot path, and
repeated sizeof() - 1 does not read so nice. The alternative would be to
hardcode the offsets.
I replaced 5 (i.e. the length link) with
sizeof(LINK_PROPERTY_TYPE_PREFIX) - 1.
If we assume that strlen(link) is always optimized away,
then certainly using strlen is equivalent and looks better.
Andreas
Alex
Re: [Qemu-devel] [PATCH] Add object_property_get_child().
On 02/17/2012 12:17 PM, Paolo Bonzini wrote:
Besides the non-triviality of the patch, how is this different from
object_property_get_link? Perhaps we should just rename that one to
object_property_get_obj, or add a function that is just a synonym.
As for the patch's (non-)triviality, I just wanted to check what counts
as trivial here.
I agree that was a bit over-reaching.
The proposed object_property_get_child() may return either
the direct child with the specified name in the composition tree,
or the value of the link with the specified name, as
object_property_get_link() indeed does.
It has exactly the same semantics as:
Object *object_property_get_child(Object *obj, const char *name,
struct Error **errp)
{
Object * result;
bool ambigious;
gchar *child_path;
/* If name contains a '/' in it, report an error and return NULL. */
child_path = g_strdup_printf(%s/%s,
object_get_canonical_path(obj), name);
result = object_resolve_path(child_path, ambigious);
g_free(child_path);
if (result == NULL) {
/* Report an error. */
return NULL;
}
if (ambigious) {
/* Report an error. */
return NULL;
}
return result;
}
but does it in a more straightforward way.
+ */
+Object *object_property_get_child(Object *obj, const char *name,
+ struct Error **errp);
+
+/**
* object_property_set:
* @obj: the object
* @v: the visitor that will be used to write the property value. This
should
diff --git a/qom/object.c b/qom/object.c
index 2de6eaf..61e775b 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -297,12 +297,86 @@ static void object_property_del_all(Object *obj)
}
}
+/*
+ * To ensure correct format checking,
+ * this function should be used only via REPORT_OBJECT_ERROR() macro.
+ *
+ * The first argument after 'obj' should be of type 'const char *'.
+ * It is ignored, and replaced by the canonical path of 'obj'.
+ */
+static void report_object_error(Error **errp, const char *fmt, Object
*obj, ...)
+GCC_FMT_ATTR(2, 4);
+static void report_object_error(Error **errp, const char *fmt, Object
*obj, ...)
+{
+gchar *path;
+va_list ap;
+
+if (errp != NULL) {
+path = object_get_canonical_path(obj);
+va_start(ap, obj);
+va_arg(ap, const char *); /* Ignore the dummy string. */
Why do you need it at all?
I think this is needed to avoid replicating code and have an easy tool
to report
object's path in errors.
+error_set(errp, fmt, path,ap);
+va_end(ap);
+g_free(path);
+}
+}
+#define REPORT_OBJECT_ERROR(errp, fmt, obj, ...) \
+do { \
+report_object_error(errp, fmt, obj, , ## __VA_ARGS__); \
+} while (0)
+#define CHILD_PROPERTY_TYPE_PREFIX child
+#define CHILD_PROPERTY_TYPE_SUFFIX
+#define LINK_PROPERTY_TYPE_PREFIX link
+#define LINK_PROPERTY_TYPE_SUFFIX
+
+static bool object_property_is_child(ObjectProperty *prop)
+{
+return (strstart(prop-type, CHILD_PROPERTY_TYPE_PREFIX, NULL) != 0);
+}
+
+static bool object_property_is_link(ObjectProperty *prop)
+{
+return (strstart(prop-type, LINK_PROPERTY_TYPE_PREFIX, NULL) != 0);
+}
+
+/* Go from LINK_PROPERTY_TYPE_PREFIX FOO LINK_PROPERTY_TYPE_SUFFIX to
FOO. */
+static gchar *link_type_to_type(const gchar *type)
+{
+return g_strndup(type[sizeof(LINK_PROPERTY_TYPE_PREFIX) - 1],
+ strlen(type)
+ - (sizeof(LINK_PROPERTY_TYPE_PREFIX) - 1)
+ - (sizeof(LINK_PROPERTY_TYPE_SUFFIX) - 1));
+}
+
+static Object *object_property_extract_child(ObjectProperty *prop,
+ bool *p_is_invalid_type)
+{
+if (p_is_invalid_type != NULL) {
+*p_is_invalid_type = false;
+}
+if (object_property_is_child(prop)) {
+return (Object *)prop-opaque;
+} else if (object_property_is_link(prop)) {
+if (prop-opaque != NULL) {
+return *(Object **)prop-opaque;
+} else {
+return NULL;
+}
+} else {
+if (p_is_invalid_type != NULL) {
+*p_is_invalid_type = true;
+}
+return NULL;
+}
+}
+
object_property_get_child should never be on a fast path. We should
avoid as much as possible peeking in the opaque. Instead, you should
just use a visitor like object_property_get_link does.
I am not sure what do you mean by should never be on a fast path.
Anyway, I did not add any peeking in the opaque. I just moved the code
from object_resolve_abs_path() into a subroutine. Fairly enough, we could
avoid looking into the link property's opaque by calling
object_property_get_link()
instead. I'll prepare a revision of the patch to this end.
Everything starting here should be a separate patch. But if you remove
object_property_extract_child, I doubt it makes as much sense to
[Qemu-devel] [PATCH v2] Revised: Add object_property_get_child().
Add object_property_get_child().
Adding a direct accessor to a child property.
In the existing implementation, object_property_get() must be used,
with with a visitor, implementing the 'type_str' callback,
receiving the child's canonical path.
In the new implementation, the child is returned directly.
For link properties, object_property_get_link() is used
to resolve the link.
Also, in the new implementation, object_property_get_child() is used
as a subroutine of object_resolve_abs_path(). This changes the way
object_resolve_abs_path() operates, moving away from directly peeking
the property's 'opaque' field to using object_property_get_link().
Thus, in the mew implementation link properties are resolved in the
same way,
as they are when an absolute path is resolved.
Errors relevant to the operation, QERR_OBJECT_PROPERTY_NOT_FOUND
and QERR_OBJECT_PROPERTY_INVALID_TYPE were added.
Also, in the new implementation, some common sense refactoring was done
in the file 'qom/object.c' in the code extracting child and link
properties.
Signed-off-by: Alexander Barabash alexander_barab...@mentor.com
diff --git a/include/qemu/object.h b/include/qemu/object.h
index ba2409d..001521d 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -711,6 +711,21 @@ int64_t object_property_get_int(Object *obj, const
char *name,
struct Error **errp);
/**
+ * object_property_get_child:
+ * @obj: the object
+ * @name: the name of the property
+ * @errp: returns an error if this function fails
+ *
+ * Returns: if this a child property, the value of the property, or NULL if
+ * an error occurs (including when the property value is not a child
property).
+ *
+ * Result's reference count does not change.
+ * Therefore, he caller is responsible for referencing the result.
+ */
+Object *object_property_get_child(Object *obj, const char *name,
+ struct Error **errp);
+
+/**
* object_property_set:
* @obj: the object
* @v: the visitor that will be used to write the property value.
This should
diff --git a/qerror.h b/qerror.h
index e26c635..45e4468 100644
--- a/qerror.h
+++ b/qerror.h
@@ -178,6 +178,12 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_NOT_SUPPORTED \
{ 'class': 'NotSupported', 'data': {} }
+#define QERR_OBJECT_PROPERTY_NOT_FOUND \
+{ 'class': 'ObjectPropertyNotFound', 'data': { 'object': %s,
'property': %s } }
+
+#define QERR_OBJECT_PROPERTY_INVALID_TYPE \
+{ 'class': 'ObjectPropertyInvalidType', 'data': { 'object': %s,
'property': %s, 'expected_type': %s } }
+
#define QERR_OPEN_FILE_FAILED \
{ 'class': 'OpenFileFailed', 'data': { 'filename': %s } }
diff --git a/qom/object.c b/qom/object.c
index 2de6eaf..ddd19e1 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -297,12 +297,64 @@ static void object_property_del_all(Object *obj)
}
}
+/*
+ * To ensure correct format checking,
+ * this function should be used only via REPORT_OBJECT_ERROR() macro.
+ *
+ * The first argument after 'obj' should be of type 'const char *'.
+ * It is ignored, and replaced by the canonical path of 'obj'.
+ */
+static void report_object_error(Error **errp, const char *fmt, Object
*obj, ...)
+GCC_FMT_ATTR(2, 4);
+static void report_object_error(Error **errp, const char *fmt, Object
*obj, ...)
+{
+gchar *path;
+va_list ap;
+
+if (errp != NULL) {
+path = object_get_canonical_path(obj);
+va_start(ap, obj);
+va_arg(ap, const char *); /* Ignore the dummy string. */
+error_set(errp, fmt, path, ap);
+va_end(ap);
+g_free(path);
+}
+}
+#define REPORT_OBJECT_ERROR(errp, fmt, obj, ...) \
+do { \
+report_object_error(errp, fmt, obj, , ## __VA_ARGS__); \
+} while (0)
+
+#define CHILD_PROPERTY_TYPE_PREFIX child
+#define CHILD_PROPERTY_TYPE_SUFFIX
+#define LINK_PROPERTY_TYPE_PREFIX link
+#define LINK_PROPERTY_TYPE_SUFFIX
+
+static bool object_property_is_child(ObjectProperty *prop)
+{
+return (strstart(prop-type, CHILD_PROPERTY_TYPE_PREFIX, NULL) != 0);
+}
+
+static bool object_property_is_link(ObjectProperty *prop)
+{
+return (strstart(prop-type, LINK_PROPERTY_TYPE_PREFIX, NULL) != 0);
+}
+
+/* Go from LINK_PROPERTY_TYPE_PREFIX FOO LINK_PROPERTY_TYPE_SUFFIX to
FOO. */
+static gchar *link_type_to_type(const gchar *type)
+{
+return g_strndup(type[sizeof(LINK_PROPERTY_TYPE_PREFIX) - 1],
+ strlen(type)
+ - (sizeof(LINK_PROPERTY_TYPE_PREFIX) - 1)
+ - (sizeof(LINK_PROPERTY_TYPE_SUFFIX) - 1));
+}
+
static void object_property_del_child(Object *obj, Object *child,
Error **errp)
{
ObjectProperty *prop;
QTAILQ_FOREACH(prop, obj-properties, node) {
-if (!strstart(prop-type, child, NULL
[Qemu-devel] [PATCH] Add object_property_get_child().
Add object_property_get_child().
Adding a direct accessor to a child property.
In the existing implementation, object_property_get() must be used,
with with a visitor, implementing the 'type_str' callback,
receiving the child's canonical path.
In the new implementation, the child is returned directly.
Link properties are resolved in the same way, as they are
when an absolute path is resolved.
Errors relevant to the operation, QERR_OBJECT_PROPERTY_NOT_FOUND
and QERR_OBJECT_PROPERTY_INVALID_TYPE were added.
Also, in the new implementation, some common sense refactoring was done
in the file 'qom/object.c' in the code extracting child and link
properties.
Signed-off-by: Alexander Barabash alexander_barab...@mentor.com
diff --git a/include/qemu/object.h b/include/qemu/object.h
index e7e32fe..190f422 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -711,6 +711,21 @@ int64_t object_property_get_int(Object *obj, const
char *name,
struct Error **errp);
/**
+ * object_property_get_child:
+ * @obj: the object
+ * @name: the name of the property
+ * @errp: returns an error if this function fails
+ *
+ * Returns: if this a child property, the value of the property, or NULL if
+ * an error occurs (including when the property value is not a child
property).
+ *
+ * Result's reference count does not change.
+ * Therefore, he caller is responsible for referencing the result.
+ */
+Object *object_property_get_child(Object *obj, const char *name,
+ struct Error **errp);
+
+/**
* object_property_set:
* @obj: the object
* @v: the visitor that will be used to write the property value.
This should
diff --git a/qerror.h b/qerror.h
index e26c635..45e4468 100644
--- a/qerror.h
+++ b/qerror.h
@@ -178,6 +178,12 @@ QError *qobject_to_qerror(const QObject *obj);
#define QERR_NOT_SUPPORTED \
{ 'class': 'NotSupported', 'data': {} }
+#define QERR_OBJECT_PROPERTY_NOT_FOUND \
+{ 'class': 'ObjectPropertyNotFound', 'data': { 'object': %s,
'property': %s } }
+
+#define QERR_OBJECT_PROPERTY_INVALID_TYPE \
+{ 'class': 'ObjectPropertyInvalidType', 'data': { 'object': %s,
'property': %s, 'expected_type': %s } }
+
#define QERR_OPEN_FILE_FAILED \
{ 'class': 'OpenFileFailed', 'data': { 'filename': %s } }
diff --git a/qom/object.c b/qom/object.c
index 2de6eaf..61e775b 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -297,12 +297,86 @@ static void object_property_del_all(Object *obj)
}
}
+/*
+ * To ensure correct format checking,
+ * this function should be used only via REPORT_OBJECT_ERROR() macro.
+ *
+ * The first argument after 'obj' should be of type 'const char *'.
+ * It is ignored, and replaced by the canonical path of 'obj'.
+ */
+static void report_object_error(Error **errp, const char *fmt, Object
*obj, ...)
+GCC_FMT_ATTR(2, 4);
+static void report_object_error(Error **errp, const char *fmt, Object
*obj, ...)
+{
+gchar *path;
+va_list ap;
+
+if (errp != NULL) {
+path = object_get_canonical_path(obj);
+va_start(ap, obj);
+va_arg(ap, const char *); /* Ignore the dummy string. */
+error_set(errp, fmt, path, ap);
+va_end(ap);
+g_free(path);
+}
+}
+#define REPORT_OBJECT_ERROR(errp, fmt, obj, ...) \
+do { \
+report_object_error(errp, fmt, obj, , ## __VA_ARGS__); \
+} while (0)
+
+#define CHILD_PROPERTY_TYPE_PREFIX child
+#define CHILD_PROPERTY_TYPE_SUFFIX
+#define LINK_PROPERTY_TYPE_PREFIX link
+#define LINK_PROPERTY_TYPE_SUFFIX
+
+static bool object_property_is_child(ObjectProperty *prop)
+{
+return (strstart(prop-type, CHILD_PROPERTY_TYPE_PREFIX, NULL) != 0);
+}
+
+static bool object_property_is_link(ObjectProperty *prop)
+{
+return (strstart(prop-type, LINK_PROPERTY_TYPE_PREFIX, NULL) != 0);
+}
+
+/* Go from LINK_PROPERTY_TYPE_PREFIX FOO LINK_PROPERTY_TYPE_SUFFIX to
FOO. */
+static gchar *link_type_to_type(const gchar *type)
+{
+return g_strndup(type[sizeof(LINK_PROPERTY_TYPE_PREFIX) - 1],
+ strlen(type)
+ - (sizeof(LINK_PROPERTY_TYPE_PREFIX) - 1)
+ - (sizeof(LINK_PROPERTY_TYPE_SUFFIX) - 1));
+}
+
+static Object *object_property_extract_child(ObjectProperty *prop,
+ bool *p_is_invalid_type)
+{
+if (p_is_invalid_type != NULL) {
+*p_is_invalid_type = false;
+}
+if (object_property_is_child(prop)) {
+return (Object *)prop-opaque;
+} else if (object_property_is_link(prop)) {
+if (prop-opaque != NULL) {
+return *(Object **)prop-opaque;
+} else {
+return NULL;
+}
+} else {
+if (p_is_invalid_type != NULL) {
+*p_is_invalid_type = true;
+}
+return NULL
[Qemu-devel] [PATCH] [TRIVIAL] Removed unused arm_sysctl_init().
Removed unused arm_sysctl_init().
Renamed arm_sysctl_init1() into arm_sysctl_init().
Signed-off-by: Alexander Barabash alexander_barab...@mentor.com
diff --git a/hw/arm_sysctl.c b/hw/arm_sysctl.c
index 149c639..5f1237b 100644
--- a/hw/arm_sysctl.c
+++ b/hw/arm_sysctl.c
@@ -378,7 +378,7 @@ static void arm_sysctl_gpio_set(void *opaque, int
line, int level)
}
}
-static int arm_sysctl_init1(SysBusDevice *dev)
+static int arm_sysctl_init(SysBusDevice *dev)
{
arm_sysctl_state *s = FROM_SYSBUS(arm_sysctl_state, dev);
@@ -389,18 +389,6 @@ static int arm_sysctl_init1(SysBusDevice *dev)
return 0;
}
-/* Legacy helper function. */
-void arm_sysctl_init(uint32_t base, uint32_t sys_id, uint32_t proc_id)
-{
-DeviceState *dev;
-
-dev = qdev_create(NULL, realview_sysctl);
-qdev_prop_set_uint32(dev, sys_id, sys_id);
-qdev_init_nofail(dev);
-qdev_prop_set_uint32(dev, proc_id, proc_id);
-sysbus_mmio_map(sysbus_from_qdev(dev), 0, base);
-}
-
static Property arm_sysctl_properties[] = {
DEFINE_PROP_UINT32(sys_id, arm_sysctl_state, sys_id, 0),
DEFINE_PROP_UINT32(proc_id, arm_sysctl_state, proc_id, 0),
@@ -412,7 +400,7 @@ static void arm_sysctl_class_init(ObjectClass
*klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
-k-init = arm_sysctl_init1;
+k-init = arm_sysctl_init;
dc-reset = arm_sysctl_reset;
dc-vmsd = vmstate_arm_sysctl;
dc-props = arm_sysctl_properties;
diff --git a/hw/primecell.h b/hw/primecell.h
index ded0446..7337c3b 100644
--- a/hw/primecell.h
+++ b/hw/primecell.h
@@ -5,9 +5,6 @@
/* Also includes some devices that are currently only used by the
ARM boards. */
-/* arm_sysctl.c */
-void arm_sysctl_init(uint32_t base, uint32_t sys_id, uint32_t proc_id);
-
/* arm_sysctl GPIO lines */
#define ARM_SYSCTL_GPIO_MMC_WPROT 0
#define ARM_SYSCTL_GPIO_MMC_CARDIN 1
[Qemu-devel] [PATCH] [TRIVIAL] Removed unused pl080_init().
Removed unused pl080_init(). Signed-off-by: Alexander Barabash alexander_barab...@mentor.com diff --git a/hw/primecell.h b/hw/primecell.h index de7d6f2..ded0446 100644 --- a/hw/primecell.h +++ b/hw/primecell.h @@ -5,9 +5,6 @@ /* Also includes some devices that are currently only used by the ARM boards. */ -/* pl080.c */ -void *pl080_init(uint32_t base, qemu_irq irq, int nchannels); - /* arm_sysctl.c */ void arm_sysctl_init(uint32_t base, uint32_t sys_id, uint32_t proc_id);
[Qemu-devel] [PATCH] [TRIVIAL] Replace object_delete() with object_unref().
Replace object_delete() with object_unref().
In the existing implementation, object_delete()
calls object_unref(), then frees the object's storage.
Running object_delete() on an object with reference count
different from one (1) causes program failure.
In the existing implementation, object_unref()
finalizes the object when its reference count becomes zero (0).
In the new implementation, object_unref()
finalizes and frees the object's storage when the reference count
becomes zero (0).
One usage of object_delete() replaced by object_unref().
Signed-off-by: Alexander Barabash alexander_barab...@mentor.com
diff --git a/hw/qdev.c b/hw/qdev.c
index f0eb3a7..891981a 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -247,7 +247,7 @@ void qdev_init_nofail(DeviceState *dev)
/* Unlink device from bus and free the structure. */
void qdev_free(DeviceState *dev)
{
-object_delete(OBJECT(dev));
+object_unref(OBJECT(dev));
}
void qdev_machine_creation_done(void)
diff --git a/include/qemu/object.h b/include/qemu/object.h
index 69cc2ab..e7e32fe 100644
--- a/include/qemu/object.h
+++ b/include/qemu/object.h
@@ -415,8 +415,9 @@ struct InterfaceInfo
* object_new:
* @typename: The name of the type of the object to instantiate.
*
- * This function will initialize a new object using heap allocated
memory. This
- * function should be paired with object_delete() to free the resources
+ * This function will initialize a new object using heap allocated memory.
+ * The object's reference count will be set to one (1) upon successful
completion.
+ * This function should be paired with object_unref() to free the resources
* associated with the object.
*
* Returns: The newly allocated and instantiated object.
@@ -427,8 +428,9 @@ Object *object_new(const char *typename);
* object_new_with_type:
* @type: The type of the object to instantiate.
*
- * This function will initialize a new object using heap allocated
memory. This
- * function should be paired with object_delete() to free the resources
+ * This function will initialize a new object using heap allocated memory.
+ * The object's reference count will be set to one (1) upon successful
completion.
+ * This function should be paired with object_unref() to free the resources
* associated with the object.
*
* Returns: The newly allocated and instantiated object.
@@ -436,15 +438,6 @@ Object *object_new(const char *typename);
Object *object_new_with_type(Type type);
/**
- * object_delete:
- * @obj: The object to free.
- *
- * Finalize an object and then free the memory associated with it.
This should
- * be paired with object_new() to free the resources associated with an
object.
- */
-void object_delete(Object *obj);
-
-/**
* object_initialize_with_type:
* @obj: A pointer to the memory to be used for the object.
* @type: The type of the object to instantiate.
@@ -573,8 +566,10 @@ void object_ref(Object *obj);
* qdef_unref:
* @obj: the object
*
- * Decrease the reference count of a object. A object cannot be freed
as long
+ * Decrease the reference count of a object. A object is not freed as long
* as its reference count is greater than zero.
+ * Once an object's reference count gets to zero (0),
+ * the object is finalized and the memory associated with it is freed.
*/
void object_unref(Object *obj);
diff --git a/qom/object.c b/qom/object.c
index 0cbd9bb..2de6eaf 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -328,7 +328,7 @@ static void object_deinit(Object *obj, TypeImpl *type)
while (obj-interfaces) {
Interface *iface_obj = obj-interfaces-data;
obj-interfaces = g_slist_delete_link(obj-interfaces,
obj-interfaces);
-object_delete(OBJECT(iface_obj));
+object_unref(OBJECT(iface_obj));
}
if (type_has_parent(type)) {
@@ -369,13 +369,6 @@ Object *object_new(const char *typename)
return object_new_with_type(ti);
}
-void object_delete(Object *obj)
-{
-object_unref(obj);
-g_assert(obj-ref == 0);
-g_free(obj);
-}
-
static bool type_is_ancestor(TypeImpl *type, TypeImpl *target_type)
{
assert(target_type);
@@ -583,6 +576,7 @@ void object_unref(Object *obj)
/* parent always holds a reference to its children */
if (obj-ref == 0) {
object_finalize(obj);
+g_free(obj);
}
}
