Re: [Qemu-devel] [PATCH target-arm v4 03/16] target-arm: cpu64: Add support for cortex-a53
Hi, On Mon, Mar 23, 2015 at 8:05 PM, Peter Crosthwaite wrote: > Similar to a53, but with different L1 I cache policy, phys addr size and ^^^ I guess a57 :) ozaki-r > different cache geometries. The cache sizes is implementation > configurable, but use these values (from Xilinx MPSoC) as a default > until cache size configurability is added. > > Reviewed-by: Alex Bennée > Signed-off-by: Peter Crosthwaite > --- > Changed since v2: > Added dtb compatible string > > target-arm/cpu64.c | 16 > 1 file changed, 16 insertions(+) > > diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c > index 3eb58c6..728d9a7 100644 > --- a/target-arm/cpu64.c > +++ b/target-arm/cpu64.c > @@ -149,6 +149,21 @@ static void aarch64_a57_initfn(Object *obj) > cpu->ccsidr[2] = 0x70ffe07a; /* 2048KB L2 cache */ > } > > +static void aarch64_a53_initfn(Object *obj) > +{ > +ARMCPU *cpu = ARM_CPU(obj); > + > +aarch64_axx_initfn(cpu); > + > +cpu->dtb_compatible = "arm,cortex-a53"; > +cpu->midr = 0x410fd034; > +cpu->ctr = 0x84448004; /* L1Ip = VIPT */ > +cpu->id_aa64mmfr0 = 0x1122; /* 40 bit physical addr */ > +cpu->ccsidr[0] = 0x700fe01a; /* 32KB L1 dcache */ > +cpu->ccsidr[1] = 0x201fe00a; /* 32KB L1 icache */ > +cpu->ccsidr[2] = 0x707fe07a; /* 1024KB L2 cache */ > +} > + > #ifdef CONFIG_USER_ONLY > static void aarch64_any_initfn(Object *obj) > { > @@ -176,6 +191,7 @@ typedef struct ARMCPUInfo { > > static const ARMCPUInfo aarch64_cpus[] = { > { .name = "cortex-a57", .initfn = aarch64_a57_initfn }, > +{ .name = "cortex-a53", .initfn = aarch64_a53_initfn }, > #ifdef CONFIG_USER_ONLY > { .name = "any", .initfn = aarch64_any_initfn }, > #endif > -- > 2.3.1.2.g90df61e.dirty >
Re: [Qemu-devel] [PATCH] target-arm: Add missing compatible property to A57
2015/03/10 21:08 "Peter Maydell" : > > On 16 February 2015 at 14:43, Ryota Ozaki wrote: > > Signed-off-by: Ryota Ozaki > > Reviewed-by: Alistair Francis > > --- > > target-arm/cpu64.c | 1 + > > 1 file changed, 1 insertion(+) > > > > Applied to target-arm.next, thanks. Thanks! ozaki-r > > -- PMM
[Qemu-devel] [PATCH] target-arm: Add missing compatible property to A57
Signed-off-by: Ryota Ozaki Reviewed-by: Alistair Francis --- target-arm/cpu64.c | 1 + 1 file changed, 1 insertion(+) diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c index 823c739..270bc2f 100644 --- a/target-arm/cpu64.c +++ b/target-arm/cpu64.c @@ -96,6 +96,7 @@ static void aarch64_a57_initfn(Object *obj) { ARMCPU *cpu = ARM_CPU(obj); +cpu->dtb_compatible = "arm,cortex-a57"; set_feature(&cpu->env, ARM_FEATURE_V8); set_feature(&cpu->env, ARM_FEATURE_VFP4); set_feature(&cpu->env, ARM_FEATURE_NEON); -- 2.3.0
Re: [Qemu-devel] [PATCH] target-arm: Add missing compatible property to A53
On Mon, Feb 16, 2015 at 9:30 AM, Alistair Francis wrote: > On Mon, Feb 16, 2015 at 12:17 AM, Ryota Ozaki wrote: >> Signed-off-by: Ryota Ozaki > > The title of your patch is incorrect, you specified A53 when you really mean > A57 Definitely. I'm sorry for my fault (I found this problem when I was trying A53 emulation...) > >> --- >> target-arm/cpu64.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c >> index 823c739..270bc2f 100644 >> --- a/target-arm/cpu64.c >> +++ b/target-arm/cpu64.c >> @@ -96,6 +96,7 @@ static void aarch64_a57_initfn(Object *obj) >> { >> ARMCPU *cpu = ARM_CPU(obj); >> >> +cpu->dtb_compatible = "arm,cortex-a57"; > > What happens without this? Recent Linux looks up the property and warns "Unknown CPU type" if it doesn't exist. Not big deal, actually. > > Once the title is fixed: > > Reviewed-by: Alistair Francis Thanks! I'm preparing a revised patch. ozaki-r > >> set_feature(&cpu->env, ARM_FEATURE_V8); >> set_feature(&cpu->env, ARM_FEATURE_VFP4); >> set_feature(&cpu->env, ARM_FEATURE_NEON); >> -- >> 2.3.0 >> >>
[Qemu-devel] [PATCH] target-arm: Add missing compatible property to A53
Signed-off-by: Ryota Ozaki --- target-arm/cpu64.c | 1 + 1 file changed, 1 insertion(+) diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c index 823c739..270bc2f 100644 --- a/target-arm/cpu64.c +++ b/target-arm/cpu64.c @@ -96,6 +96,7 @@ static void aarch64_a57_initfn(Object *obj) { ARMCPU *cpu = ARM_CPU(obj); +cpu->dtb_compatible = "arm,cortex-a57"; set_feature(&cpu->env, ARM_FEATURE_V8); set_feature(&cpu->env, ARM_FEATURE_VFP4); set_feature(&cpu->env, ARM_FEATURE_NEON); -- 2.3.0
Re: [Qemu-devel] [PATCH 0/3] Intruduce qemu-ga-client
Hi Michael On Thu, Sep 20, 2012 at 1:40 PM, Michael Tokarev wrote: > On 20.09.2012 00:20, Luiz Capitulino wrote: >> On Fri, 14 Sep 2012 21:44:19 +0900 >> Ryota Ozaki wrote: >> >>> This patch series adds qemu-ga-client that is a command line >>> script to use guest agent functions easily. > > Can't this be implemented in the agent itself, without a need > for a wrapper? Note the wrapper depends on python which is > not present on every guest, while relatively small C code is > much easier to have available. This script is intended to run on host side like qmp-shell, so we cannot integrate it in the guest agent. Regards, ozaki-r > > Thanks, > > /mjt
[Qemu-devel] [PATCH 0/3] Intruduce qemu-ga-client
This patch series adds qemu-ga-client that is a command line script to use guest agent functions easily. Ryota Ozaki (3): Make negotiation optional in QEMUMonitorProtocol Support settimeout in QEMUMonitorProtocol Add qemu-ga-client script QMP/qemu-ga-client | 299 + QMP/qmp.py | 12 ++- 2 files changed, 308 insertions(+), 3 deletions(-) create mode 100755 QMP/qemu-ga-client -- 1.7.12
[Qemu-devel] [PATCH 3/3] Add qemu-ga-client script
This is an easy-to-use QEMU guest agent client written in Python. It simply provides commands to call guest agent functions like ping, fsfreeze and shutdown. Additionally, it provides extra useful commands, e.g, cat, ifconfig and reboot, by using guet agent functions. Examples: $ export QGA_CLIENT_ADDRESS=/tmp/qga.sock $ qemu-ga-client ping $ qemu-ga-client cat /etc/resolv.conf # Generated by NetworkManager nameserver 10.0.2.3 $ qemu-ga-client fsfreeze status thawed $ qemu-ga-client fsfreeze freeze 2 filesystems frozen The script communicates with a guest agent by means of qmp.QEMUMonitorProtocol. Every commands are called with timeout (3 sec.) to avoid blocking. The script always calls sync command prior to issuing an actual command (except for ping which doesn't need sync). Signed-off-by: Ryota Ozaki --- QMP/qemu-ga-client | 299 + 1 file changed, 299 insertions(+) create mode 100755 QMP/qemu-ga-client diff --git a/QMP/qemu-ga-client b/QMP/qemu-ga-client new file mode 100755 index 000..46676c3 --- /dev/null +++ b/QMP/qemu-ga-client @@ -0,0 +1,299 @@ +#!/usr/bin/python + +# QEMU Guest Agent Client +# +# Copyright (C) 2012 Ryota Ozaki +# +# This work is licensed under the terms of the GNU GPL, version 2. See +# the COPYING file in the top-level directory. +# +# Usage: +# +# Start QEMU with: +# +# # qemu [...] -chardev socket,path=/tmp/qga.sock,server,nowait,id=qga0 \ +# -device virtio-serial -device virtserialport,chardev=qga0,name=org.qemu.guest_agent.0 +# +# Run the script: +# +# $ qemu-ga-client --address=/tmp/qga.sock [args...] +# +# or +# +# $ export QGA_CLIENT_ADDRESS=/tmp/qga.sock +# $ qemu-ga-client [args...] +# +# For example: +# +# $ qemu-ga-client cat /etc/resolv.conf +# # Generated by NetworkManager +# nameserver 10.0.2.3 +# $ qemu-ga-client fsfreeze status +# thawed +# $ qemu-ga-client fsfreeze freeze +# 2 filesystems frozen +# +# See also: http://wiki.qemu.org/Features/QAPI/GuestAgent +# + +import base64 +import random + +import qmp + + +class QemuGuestAgent(qmp.QEMUMonitorProtocol): +def __getattr__(self, name): +def wrapper(**kwds): +return self.command('guest-' + name.replace('_', '-'), **kwds) +return wrapper + + +class QemuGuestAgentClient: +error = QemuGuestAgent.error + +def __init__(self, address): +self.qga = QemuGuestAgent(address) +self.qga.connect(negotiate=False) + +def sync(self, timeout=3): +# Avoid being blocked forever +if not self.ping(timeout): +raise EnvironmentError('Agent seems not alive') +uid = random.randint(0, (1 << 32) - 1) +while True: +ret = self.qga.sync(id=uid) +if isinstance(ret, int) and int(ret) == uid: +break + +def __file_read_all(self, handle): +eof = False +data = '' +while not eof: +ret = self.qga.file_read(handle=handle, count=1024) +_data = base64.b64decode(ret['buf-b64']) +data += _data +eof = ret['eof'] +return data + +def read(self, path): +handle = self.qga.file_open(path=path) +try: +data = self.__file_read_all(handle) +finally: +self.qga.file_close(handle=handle) +return data + +def info(self): +info = self.qga.info() + +msgs = [] +msgs.append('version: ' + info['version']) +msgs.append('supported_commands:') +enabled = [c['name'] for c in info['supported_commands'] if c['enabled']] +msgs.append('\tenabled: ' + ', '.join(enabled)) +disabled = [c['name'] for c in info['supported_commands'] if not c['enabled']] +msgs.append('\tdisabled: ' + ', '.join(disabled)) + +return '\n'.join(msgs) + +def __gen_ipv4_netmask(self, prefixlen): +mask = int('1' * prefixlen + '0' * (32 - prefixlen), 2) +return '.'.join([str(mask >> 24), + str((mask >> 16) & 0xff), + str((mask >> 8) & 0xff), + str(mask & 0xff)]) + +def ifconfig(self): +nifs = self.qga.network_get_interfaces() + +msgs = [] +for nif in nifs: +msgs.append(nif['name'] + ':') +if 'ip-addresses' in nif: +for ipaddr in nif['ip-addresses']: +if ipaddr['ip-address-type'] == 'ipv4': +addr = ipaddr['ip-address'] +mask = self.__gen_ipv4_netmask(int(ipaddr['prefix'])) +
[Qemu-devel] [PATCH 1/3] Make negotiation optional in QEMUMonitorProtocol
This is a preparation for qemu-ga-client which uses QEMUMonitorProtocol class. The class tries to negotiate capabilities on connect, however, qemu-ga doesn't suppose it and fails. This change makes the negotiation optional, though it's still performed by default for compatibility. Signed-off-by: Ryota Ozaki --- QMP/qmp.py | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/QMP/qmp.py b/QMP/qmp.py index 36ecc1d..5a573e1 100644 --- a/QMP/qmp.py +++ b/QMP/qmp.py @@ -49,7 +49,6 @@ class QEMUMonitorProtocol: return socket.socket(family, socket.SOCK_STREAM) def __negotiate_capabilities(self): -self.__sockfile = self.__sock.makefile() greeting = self.__json_read() if greeting is None or not greeting.has_key('QMP'): raise QMPConnectError @@ -73,7 +72,7 @@ class QEMUMonitorProtocol: error = socket.error -def connect(self): +def connect(self, negotiate=True): """ Connect to the QMP Monitor and perform capabilities negotiation. @@ -83,7 +82,9 @@ class QEMUMonitorProtocol: @raise QMPCapabilitiesError if fails to negotiate capabilities """ self.__sock.connect(self.__address) -return self.__negotiate_capabilities() +self.__sockfile = self.__sock.makefile() +if negotiate: +return self.__negotiate_capabilities() def accept(self): """ -- 1.7.12
[Qemu-devel] [PATCH 2/3] Support settimeout in QEMUMonitorProtocol
This method is used in the following qemu-ga-client script to implement non-blocking operations. Signed-off-by: Ryota Ozaki --- QMP/qmp.py | 5 + 1 file changed, 5 insertions(+) diff --git a/QMP/qmp.py b/QMP/qmp.py index 5a573e1..33c7d36 100644 --- a/QMP/qmp.py +++ b/QMP/qmp.py @@ -162,3 +162,8 @@ class QEMUMonitorProtocol: def close(self): self.__sock.close() self.__sockfile.close() + +timeout = socket.timeout + +def settimeout(self, timeout): +self.__sock.settimeout(timeout) -- 1.7.12
[Qemu-devel] [PATCH] qemu-nbd: Improve error reporting
- use err(3) instead of errx(3) if errno is available to report why failed - let fail prior to daemon(3) if opening a nbd file is likely to fail after daemonizing to avoid silent failure exit - add missing 'ret = 1' when unix_socket_outgoing failed Signed-off-by: Ryota Ozaki --- qemu-nbd.c | 34 -- 1 files changed, 24 insertions(+), 10 deletions(-) diff --git a/qemu-nbd.c b/qemu-nbd.c index 25aa913..4e607cf 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -112,9 +112,12 @@ static int find_partition(BlockDriverState *bs, int partition, uint8_t data[512]; int i; int ext_partnum = 4; +int ret; -if (bdrv_read(bs, 0, data, 1)) -errx(EXIT_FAILURE, "error while reading"); +if ((ret = bdrv_read(bs, 0, data, 1)) < 0) { +errno = -ret; +err(EXIT_FAILURE, "error while reading"); +} if (data[510] != 0x55 || data[511] != 0xaa) { errno = -EINVAL; @@ -132,8 +135,10 @@ static int find_partition(BlockDriverState *bs, int partition, uint8_t data1[512]; int j; -if (bdrv_read(bs, mbr[i].start_sector_abs, data1, 1)) -errx(EXIT_FAILURE, "error while reading"); +if ((ret = bdrv_read(bs, mbr[i].start_sector_abs, data1, 1)) < 0) { +errno = -ret; +err(EXIT_FAILURE, "error while reading"); +} for (j = 0; j < 4; j++) { read_partition(&data1[446 + 16 * j], &ext[j]); @@ -316,7 +321,7 @@ int main(int argc, char **argv) if (disconnect) { fd = open(argv[optind], O_RDWR); if (fd == -1) -errx(EXIT_FAILURE, "Cannot open %s", argv[optind]); +err(EXIT_FAILURE, "Cannot open %s", argv[optind]); nbd_disconnect(fd); @@ -333,23 +338,30 @@ int main(int argc, char **argv) if (bs == NULL) return 1; -if (bdrv_open(bs, argv[optind], flags, NULL) < 0) -return 1; +if ((ret = bdrv_open(bs, argv[optind], flags, NULL)) < 0) { +errno = -ret; +err(EXIT_FAILURE, "Failed to bdrv_open '%s'", argv[optind]); +} fd_size = bs->total_sectors * 512; if (partition != -1 && find_partition(bs, partition, &dev_offset, &fd_size)) -errx(EXIT_FAILURE, "Could not find partition %d", partition); +err(EXIT_FAILURE, "Could not find partition %d", partition); if (device) { pid_t pid; int sock; +/* want to fail before daemonizing */ +if (access(device, R_OK|W_OK) == -1) { +err(EXIT_FAILURE, "Could not access '%s'", device); +} + if (!verbose) { /* detach client and server */ if (daemon(0, 0) == -1) { -errx(EXIT_FAILURE, "Failed to daemonize"); +err(EXIT_FAILURE, "Failed to daemonize"); } } @@ -372,8 +384,10 @@ int main(int argc, char **argv) do { sock = unix_socket_outgoing(socket); if (sock == -1) { -if (errno != ENOENT && errno != ECONNREFUSED) +if (errno != ENOENT && errno != ECONNREFUSED) { +ret = 1; goto out; +} sleep(1); /* wait children */ } } while (sock == -1); -- 1.6.5.2
Re: [Qemu-devel] [PATCH 3/3] qemu-nbd: Improve error reporting
On Mon, Mar 29, 2010 at 8:03 PM, Kevin Wolf wrote: > Am 28.03.2010 19:07, schrieb Ryota Ozaki: >> - use err(3) instead of errx(3) if errno is available >> to report why failed >> - let fail prior to daemon(3) if opening a nbd file >> is likely to fail after daemonizing to avoid silent >> failure exit >> - add missing 'ret = 1' when unix_socket_outgoing failed >> >> Signed-off-by: Ryota Ozaki > > Acked-by: Kevin Wolf > >> @@ -343,25 +343,31 @@ int main(int argc, char **argv) >> return 1; >> } >> >> - if (bdrv_open(bs, argv[optind], flags) < 0) { >> - return 1; >> + if ((ret = bdrv_open(bs, argv[optind], flags)) < 0) { >> + errno = -ret; >> + err(EXIT_FAILURE, "Failed to bdrv_open '%s'", argv[optind]); >> } > > If you do it like this, you could do the change for even more errors, > like the ones returned by bdrv_read. But that doesn't make this patch > less correct, of course. Indeed. So I will include the changes for bdrv_read as well in the next round. Thanks, ozaki-r > > Kevin >
Re: [Qemu-devel] [PATCH 2/3] qemu-nbd: Extend read-only option to nbd device file
On Mon, Mar 29, 2010 at 7:54 PM, Kevin Wolf wrote: > Am 28.03.2010 19:07, schrieb Ryota Ozaki: >> This patch allows to operate on nbd device file >> without write permission for the file if read-only >> option is specified. >> >> Signed-off-by: Ryota Ozaki > > The help for -r should be changed, too. Currently it says: > > -r, --read-only export read-only Indeed. > >> --- >> qemu-nbd.c | 10 +- >> 1 files changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/qemu-nbd.c b/qemu-nbd.c >> index 00b8896..7ef409f 100644 >> --- a/qemu-nbd.c >> +++ b/qemu-nbd.c >> @@ -162,7 +162,7 @@ static int find_partition(BlockDriverState *bs, int >> partition, >> return -1; >> } >> >> -static void show_parts(const char *device) >> +static void show_parts(const char *device, bool readonly) >> { >> if (fork() == 0) { >> int nbd; >> @@ -172,7 +172,7 @@ static void show_parts(const char *device) >> * but remember to load the module with max_part != 0 : >> * modprobe nbd max_part=63 >> */ >> - nbd = open(device, O_RDWR); >> + nbd = open(device, readonly ? O_RDONLY : O_RDWR); >> if (nbd != -1) { >> close(nbd); >> } > > Can't we always use O_RDONLY here? Assuming that this is enough to > trigger a partition table update, I haven't tested it. But if it's not > enough, wouldn't be enough for readonly either. You're right. It works always with O_RDONLY. So we can remove the condition expression. However, I will drop this patch because I found I misunderstood about nbd. The patch intended to be for non-root users, but it does not make sense because nbd requires CAP_SYS_ADMIN. So I hold it until I understand well and focus on the last patch at first. I'm sorry for messing up. Thanks, ozaki-r > > Kevin >
Re: [Qemu-devel] [PATCH 1/3] qemu-nbd: Fix coding style
Hi Kevin On Mon, Mar 29, 2010 at 7:44 PM, Kevin Wolf wrote: > Am 28.03.2010 19:07, schrieb Ryota Ozaki: >> Follow "Every indented statement is braced; even if the block >> contains just one statement." described in CODING_STYLE. >> >> Signed-off-by: Ryota Ozaki > > Usually we fix the coding style in places that need to be changed > anyway, but avoid touching things only for style cleanup (it makes git > blame less meaningful, for example). Thank you for reviewing and I'm sorry I've stuck for long time... I follow the policy and will drop the patch. Thanks, ozaki-r > > Kevin >
[Qemu-devel] [PATCH 3/3] qemu-nbd: Improve error reporting
- use err(3) instead of errx(3) if errno is available to report why failed - let fail prior to daemon(3) if opening a nbd file is likely to fail after daemonizing to avoid silent failure exit - add missing 'ret = 1' when unix_socket_outgoing failed Signed-off-by: Ryota Ozaki --- qemu-nbd.c | 17 - 1 files changed, 12 insertions(+), 5 deletions(-) diff --git a/qemu-nbd.c b/qemu-nbd.c index 7ef409f..d3e1814 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -324,7 +324,7 @@ int main(int argc, char **argv) if (disconnect) { fd = open(argv[optind], readonly ? O_RDONLY : O_RDWR); if (fd == -1) { -errx(EXIT_FAILURE, "Cannot open %s", argv[optind]); +err(EXIT_FAILURE, "Cannot open %s", argv[optind]); } nbd_disconnect(fd); @@ -343,25 +343,31 @@ int main(int argc, char **argv) return 1; } -if (bdrv_open(bs, argv[optind], flags) < 0) { -return 1; +if ((ret = bdrv_open(bs, argv[optind], flags)) < 0) { +errno = -ret; +err(EXIT_FAILURE, "Failed to bdrv_open '%s'", argv[optind]); } fd_size = bs->total_sectors * 512; if (partition != -1 && find_partition(bs, partition, &dev_offset, &fd_size)) { -errx(EXIT_FAILURE, "Could not find partition %d", partition); +err(EXIT_FAILURE, "Could not find partition %d", partition); } if (device) { pid_t pid; int sock; +/* want to fail before daemonizing */ +if (access(device, readonly ? R_OK : R_OK|W_OK) == -1) { +err(EXIT_FAILURE, "Could not access '%s'", device); +} + if (!verbose) { /* detach client and server */ if (daemon(0, 0) == -1) { -errx(EXIT_FAILURE, "Failed to daemonize"); +err(EXIT_FAILURE, "Failed to daemonize"); } } @@ -386,6 +392,7 @@ int main(int argc, char **argv) sock = unix_socket_outgoing(socket); if (sock == -1) { if (errno != ENOENT && errno != ECONNREFUSED) { +ret = 1; goto out; } sleep(1); /* wait children */ -- 1.6.5.2
[Qemu-devel] [PATCH 2/3] qemu-nbd: Extend read-only option to nbd device file
This patch allows to operate on nbd device file without write permission for the file if read-only option is specified. Signed-off-by: Ryota Ozaki --- qemu-nbd.c | 10 +- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/qemu-nbd.c b/qemu-nbd.c index 00b8896..7ef409f 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -162,7 +162,7 @@ static int find_partition(BlockDriverState *bs, int partition, return -1; } -static void show_parts(const char *device) +static void show_parts(const char *device, bool readonly) { if (fork() == 0) { int nbd; @@ -172,7 +172,7 @@ static void show_parts(const char *device) * but remember to load the module with max_part != 0 : * modprobe nbd max_part=63 */ -nbd = open(device, O_RDWR); +nbd = open(device, readonly ? O_RDONLY : O_RDWR); if (nbd != -1) { close(nbd); } @@ -322,7 +322,7 @@ int main(int argc, char **argv) } if (disconnect) { -fd = open(argv[optind], O_RDWR); +fd = open(argv[optind], readonly ? O_RDONLY : O_RDWR); if (fd == -1) { errx(EXIT_FAILURE, "Cannot open %s", argv[optind]); } @@ -392,7 +392,7 @@ int main(int argc, char **argv) } } while (sock == -1); -fd = open(device, O_RDWR); +fd = open(device, readonly ? O_RDONLY : O_RDWR); if (fd == -1) { ret = 1; goto out; @@ -415,7 +415,7 @@ int main(int argc, char **argv) /* update partition table */ -show_parts(device); +show_parts(device, readonly); nbd_client(fd, sock); close(fd); -- 1.6.5.2
[Qemu-devel] [PATCH 1/3] qemu-nbd: Fix coding style
Follow "Every indented statement is braced; even if the block contains just one statement." described in CODING_STYLE. Signed-off-by: Ryota Ozaki --- qemu-nbd.c | 60 1 files changed, 40 insertions(+), 20 deletions(-) diff --git a/qemu-nbd.c b/qemu-nbd.c index 6d854d3..00b8896 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -113,8 +113,9 @@ static int find_partition(BlockDriverState *bs, int partition, int i; int ext_partnum = 4; -if (bdrv_read(bs, 0, data, 1)) +if (bdrv_read(bs, 0, data, 1)) { errx(EXIT_FAILURE, "error while reading"); +} if (data[510] != 0x55 || data[511] != 0xaa) { errno = -EINVAL; @@ -124,21 +125,24 @@ static int find_partition(BlockDriverState *bs, int partition, for (i = 0; i < 4; i++) { read_partition(&data[446 + 16 * i], &mbr[i]); -if (!mbr[i].nb_sectors_abs) +if (!mbr[i].nb_sectors_abs) { continue; +} if (mbr[i].system == 0xF || mbr[i].system == 0x5) { struct partition_record ext[4]; uint8_t data1[512]; int j; -if (bdrv_read(bs, mbr[i].start_sector_abs, data1, 1)) +if (bdrv_read(bs, mbr[i].start_sector_abs, data1, 1)) { errx(EXIT_FAILURE, "error while reading"); +} for (j = 0; j < 4; j++) { read_partition(&data1[446 + 16 * j], &ext[j]); -if (!ext[j].nb_sectors_abs) +if (!ext[j].nb_sectors_abs) { continue; +} if ((ext_partnum + j + 1) == partition) { *offset = (uint64_t)ext[j].start_sector_abs << 9; @@ -169,8 +173,9 @@ static void show_parts(const char *device) * modprobe nbd max_part=63 */ nbd = open(device, O_RDWR); -if (nbd != -1) +if (nbd != -1) { close(nbd); +} exit(0); } } @@ -262,15 +267,18 @@ int main(int argc, char **argv) break; case 'P': partition = strtol(optarg, &end, 0); -if (*end) +if (*end) { errx(EXIT_FAILURE, "Invalid partition `%s'", optarg); -if (partition < 1 || partition > 8) +} +if (partition < 1 || partition > 8) { errx(EXIT_FAILURE, "Invalid partition %d", partition); +} break; case 'k': socket = optarg; -if (socket[0] != '/') +if (socket[0] != '/') { errx(EXIT_FAILURE, "socket path must be absolute\n"); +} break; case 'd': disconnect = true; @@ -315,8 +323,9 @@ int main(int argc, char **argv) if (disconnect) { fd = open(argv[optind], O_RDWR); -if (fd == -1) +if (fd == -1) { errx(EXIT_FAILURE, "Cannot open %s", argv[optind]); +} nbd_disconnect(fd); @@ -330,17 +339,20 @@ int main(int argc, char **argv) bdrv_init(); bs = bdrv_new("hda"); -if (bs == NULL) +if (bs == NULL) { return 1; +} -if (bdrv_open(bs, argv[optind], flags) < 0) +if (bdrv_open(bs, argv[optind], flags) < 0) { return 1; +} fd_size = bs->total_sectors * 512; if (partition != -1 && -find_partition(bs, partition, &dev_offset, &fd_size)) +find_partition(bs, partition, &dev_offset, &fd_size)) { errx(EXIT_FAILURE, "Could not find partition %d", partition); +} if (device) { pid_t pid; @@ -360,8 +372,9 @@ int main(int argc, char **argv) } pid = fork(); -if (pid < 0) +if (pid < 0) { return 1; +} if (pid != 0) { off_t size; size_t blocksize; @@ -372,8 +385,9 @@ int main(int argc, char **argv) do { sock = unix_socket_outgoing(socket); if (sock == -1) { -if (errno != ENOENT && errno != ECONNREFUSED) +if (errno != ENOENT && errno != ECONNREFUSED) { goto out; +} sleep(1); /* wait children */ } } while (sock == -1); @@ -422,14 +436,16 @@ int main(int argc, char **argv) sharing_fds[0] = tcp_socket_incoming(bindto, port); } -if (sharing_fds[0] == -1) +if (sharing_fds[0] == -1) { return 1; +} max_fd = sharing_fds[0]; nb_fds++; data = qemu_memalign(512, NBD_BUFFER_SIZE); -if (data == NULL) +if (data ==
Re: [Qemu-devel] [PATCH 3/3] qemu-nbd: Improve error reporting
On Sat, Mar 27, 2010 at 10:02 PM, Aurelien Jarno wrote: > On Sat, Mar 20, 2010 at 03:23:24PM +0900, Ryota Ozaki wrote: >> - use err(3) instead of errx(3) if errno is available >> to report why failed >> - let fail prior to daemon(3) if opening a nbd file >> is likely to fail after daemonizing to avoid silent >> failure exit >> >> Signed-off-by: Ryota Ozaki >> --- >> qemu-nbd.c | 16 +++- >> 1 files changed, 11 insertions(+), 5 deletions(-) >> >> diff --git a/qemu-nbd.c b/qemu-nbd.c >> index 6d854d3..8fb8cc3 100644 >> --- a/qemu-nbd.c >> +++ b/qemu-nbd.c >> @@ -316,7 +316,7 @@ int main(int argc, char **argv) >> if (disconnect) { >> fd = open(argv[optind], O_RDWR); >> if (fd == -1) >> - errx(EXIT_FAILURE, "Cannot open %s", argv[optind]); >> + err(EXIT_FAILURE, "Cannot open %s", argv[optind]); >> >> nbd_disconnect(fd); >> >> @@ -333,23 +333,29 @@ int main(int argc, char **argv) >> if (bs == NULL) >> return 1; >> >> - if (bdrv_open(bs, argv[optind], flags) < 0) >> - return 1; >> + if ((ret = bdrv_open(bs, argv[optind], flags)) < 0) { >> + errno = -ret; >> + err(EXIT_FAILURE, "Failed to bdrv_open '%s'", argv[optind]); >> + } >> >> fd_size = bs->total_sectors * 512; >> >> if (partition != -1 && >> find_partition(bs, partition, &dev_offset, &fd_size)) >> - errx(EXIT_FAILURE, "Could not find partition %d", partition); >> + err(EXIT_FAILURE, "Could not find partition %d", partition); >> >> if (device) { >> pid_t pid; >> int sock; >> >> + /* want to fail before daemonizing */ >> + if (access(device, R_OK|W_OK) == -1) >> + err(EXIT_FAILURE, "Could not access '%s'", device); >> + > > First of all you need to put this line between curly braces. Secondly, Oh, sorry. > qemu-nbd as a read-only option to export a block device as read-only. > The test has to be improved to also take care of that. I guess the option is intended to be only for disk image, because open(2) for nbd device file doesn't care about the option. Nonetheless, extending the option to operations to nbd device file makes sense, because the option allows to open nbd device file without write permission. So I'll correct it as well as fixing the problems you pointed out. Thanks, ozaki-r > >> if (!verbose) { >> /* detach client and server */ >> if (daemon(0, 0) == -1) { >> - errx(EXIT_FAILURE, "Failed to daemonize"); >> + err(EXIT_FAILURE, "Failed to daemonize"); >> } >> } >> >> -- >> 1.6.5.2 >> >> >> >> > > -- > Aurelien Jarno GPG: 1024D/F1BCDB73 > aurel...@aurel32.net http://www.aurel32.net >
[Qemu-devel] [PATCH] qemu-io: Fix return value handling of bdrv_open
bdrv_open may return -errno so we have to check if the return value is '< 0', not '== -1'. Signed-off-by: Ryota Ozaki --- qemu-io.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/qemu-io.c b/qemu-io.c index b2f2f5a..2f195bf 100644 --- a/qemu-io.c +++ b/qemu-io.c @@ -1284,7 +1284,7 @@ static int openfile(char *name, int flags, int growable) flags |= BDRV_O_FILE; } - if (bdrv_open(bs, name, flags) == -1) { + if (bdrv_open(bs, name, flags) < 0) { fprintf(stderr, "%s: can't open device %s\n", progname, name); bs = NULL; return 1; -- 1.6.5.2
Re: [Qemu-devel] [PATCH 1/3] qemu-nbd: Fix return value handling of bdrv_open
On Sat, Mar 20, 2010 at 4:01 PM, Markus Armbruster wrote: > Ryota Ozaki writes: > >> bdrv_open may return -errno so we have to check >> if the return value is '< 0', not '== -1'. >> >> Signed-off-by: Ryota Ozaki >> --- >> qemu-nbd.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/qemu-nbd.c b/qemu-nbd.c >> index a393583..b89c361 100644 >> --- a/qemu-nbd.c >> +++ b/qemu-nbd.c >> @@ -333,7 +333,7 @@ int main(int argc, char **argv) >> if (bs == NULL) >> return 1; >> >> - if (bdrv_open(bs, argv[optind], flags) == -1) >> + if (bdrv_open(bs, argv[optind], flags) < 0) >> return 1; >> >> fd_size = bs->total_sectors * 512; > > Same bug in qemu-io.c. Could you fix that as well? > OK. I will. Thanks, ozaki-r
[Qemu-devel] [PATCH 3/3] qemu-nbd: Improve error reporting
- use err(3) instead of errx(3) if errno is available to report why failed - let fail prior to daemon(3) if opening a nbd file is likely to fail after daemonizing to avoid silent failure exit Signed-off-by: Ryota Ozaki --- qemu-nbd.c | 16 +++- 1 files changed, 11 insertions(+), 5 deletions(-) diff --git a/qemu-nbd.c b/qemu-nbd.c index 6d854d3..8fb8cc3 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -316,7 +316,7 @@ int main(int argc, char **argv) if (disconnect) { fd = open(argv[optind], O_RDWR); if (fd == -1) -errx(EXIT_FAILURE, "Cannot open %s", argv[optind]); +err(EXIT_FAILURE, "Cannot open %s", argv[optind]); nbd_disconnect(fd); @@ -333,23 +333,29 @@ int main(int argc, char **argv) if (bs == NULL) return 1; -if (bdrv_open(bs, argv[optind], flags) < 0) -return 1; +if ((ret = bdrv_open(bs, argv[optind], flags)) < 0) { +errno = -ret; +err(EXIT_FAILURE, "Failed to bdrv_open '%s'", argv[optind]); +} fd_size = bs->total_sectors * 512; if (partition != -1 && find_partition(bs, partition, &dev_offset, &fd_size)) -errx(EXIT_FAILURE, "Could not find partition %d", partition); +err(EXIT_FAILURE, "Could not find partition %d", partition); if (device) { pid_t pid; int sock; +/* want to fail before daemonizing */ +if (access(device, R_OK|W_OK) == -1) +err(EXIT_FAILURE, "Could not access '%s'", device); + if (!verbose) { /* detach client and server */ if (daemon(0, 0) == -1) { -errx(EXIT_FAILURE, "Failed to daemonize"); +err(EXIT_FAILURE, "Failed to daemonize"); } } -- 1.6.5.2
[Qemu-devel] [PATCH 2/3] qemu-nbd: Fix invalid usage of the first argument of errx
errx takes the exit status of a process as the first argument. Passing errno to it is wrong. Instead the patch lets errx take EXIT_FAILURE. Signed-off-by: Ryota Ozaki --- qemu-nbd.c | 34 +- 1 files changed, 17 insertions(+), 17 deletions(-) diff --git a/qemu-nbd.c b/qemu-nbd.c index b89c361..6d854d3 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -114,7 +114,7 @@ static int find_partition(BlockDriverState *bs, int partition, int ext_partnum = 4; if (bdrv_read(bs, 0, data, 1)) -errx(EINVAL, "error while reading"); +errx(EXIT_FAILURE, "error while reading"); if (data[510] != 0x55 || data[511] != 0xaa) { errno = -EINVAL; @@ -133,7 +133,7 @@ static int find_partition(BlockDriverState *bs, int partition, int j; if (bdrv_read(bs, mbr[i].start_sector_abs, data1, 1)) -errx(EINVAL, "error while reading"); +errx(EXIT_FAILURE, "error while reading"); for (j = 0; j < 4; j++) { read_partition(&data1[446 + 16 * j], &ext[j]); @@ -240,20 +240,20 @@ int main(int argc, char **argv) case 'p': li = strtol(optarg, &end, 0); if (*end) { -errx(EINVAL, "Invalid port `%s'", optarg); +errx(EXIT_FAILURE, "Invalid port `%s'", optarg); } if (li < 1 || li > 65535) { -errx(EINVAL, "Port out of range `%s'", optarg); +errx(EXIT_FAILURE, "Port out of range `%s'", optarg); } port = (uint16_t)li; break; case 'o': dev_offset = strtoll (optarg, &end, 0); if (*end) { -errx(EINVAL, "Invalid offset `%s'", optarg); +errx(EXIT_FAILURE, "Invalid offset `%s'", optarg); } if (dev_offset < 0) { -errx(EINVAL, "Offset must be positive `%s'", optarg); +errx(EXIT_FAILURE, "Offset must be positive `%s'", optarg); } break; case 'r': @@ -263,14 +263,14 @@ int main(int argc, char **argv) case 'P': partition = strtol(optarg, &end, 0); if (*end) -errx(EINVAL, "Invalid partition `%s'", optarg); +errx(EXIT_FAILURE, "Invalid partition `%s'", optarg); if (partition < 1 || partition > 8) -errx(EINVAL, "Invalid partition %d", partition); +errx(EXIT_FAILURE, "Invalid partition %d", partition); break; case 'k': socket = optarg; if (socket[0] != '/') -errx(EINVAL, "socket path must be absolute\n"); +errx(EXIT_FAILURE, "socket path must be absolute\n"); break; case 'd': disconnect = true; @@ -281,10 +281,10 @@ int main(int argc, char **argv) case 'e': shared = strtol(optarg, &end, 0); if (*end) { -errx(EINVAL, "Invalid shared device number '%s'", optarg); +errx(EXIT_FAILURE, "Invalid shared device number '%s'", optarg); } if (shared < 1) { -errx(EINVAL, "Shared device number must be greater than 0\n"); +errx(EXIT_FAILURE, "Shared device number must be greater than 0\n"); } break; case 't': @@ -302,13 +302,13 @@ int main(int argc, char **argv) exit(0); break; case '?': -errx(EINVAL, "Try `%s --help' for more information.", +errx(EXIT_FAILURE, "Try `%s --help' for more information.", argv[0]); } } if ((argc - optind) != 1) { -errx(EINVAL, "Invalid number of argument.\n" +errx(EXIT_FAILURE, "Invalid number of argument.\n" "Try `%s --help' for more information.", argv[0]); } @@ -316,7 +316,7 @@ int main(int argc, char **argv) if (disconnect) { fd = open(argv[optind], O_RDWR); if (fd == -1) -errx(errno, "Cannot open %s", argv[optind]); +errx(EXIT_FAILURE, "Cannot open %s", argv[optind]); nbd_disconnect(fd); @@ -340,7 +340,7 @@ int main(int argc, char **argv) if (partition != -1 && find_partition(bs, partition, &dev_offset, &fd_size)) -errx(errno, &qu
[Qemu-devel] [PATCH 1/3] qemu-nbd: Fix return value handling of bdrv_open
bdrv_open may return -errno so we have to check if the return value is '< 0', not '== -1'. Signed-off-by: Ryota Ozaki --- qemu-nbd.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/qemu-nbd.c b/qemu-nbd.c index a393583..b89c361 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -333,7 +333,7 @@ int main(int argc, char **argv) if (bs == NULL) return 1; -if (bdrv_open(bs, argv[optind], flags) == -1) +if (bdrv_open(bs, argv[optind], flags) < 0) return 1; fd_size = bs->total_sectors * 512; -- 1.6.5.2
Re: [Qemu-devel] [PATCH 2/2] qemu-nbd: Improve error reporting
On Sat, Mar 20, 2010 at 2:26 PM, malc wrote: > On Sat, 20 Mar 2010, Ryota Ozaki wrote: > >> - use err(3) instead of errx(3) if errno is available >> to report why failed >> - let fail prior to daemon(3) if opening a nbd file >> is likely to fail after daemonizing to avoid silent >> failure exit > > I'm under impression that you and the original author of this > code is misunderstading how err[x] works, first argument is > what will be used to exit the process and has little to do > with POSIX/C error codes. Oh, you're right. I'll revise and resend the patch. (and I'll add missing Signed-off-by fields) Thanks, ozaki-r > >> --- >> qemu-nbd.c | 16 +++- >> 1 files changed, 11 insertions(+), 5 deletions(-) >> >> diff --git a/qemu-nbd.c b/qemu-nbd.c >> index b89c361..5f10ff0 100644 >> --- a/qemu-nbd.c >> +++ b/qemu-nbd.c >> @@ -316,7 +316,7 @@ int main(int argc, char **argv) >> if (disconnect) { >> fd = open(argv[optind], O_RDWR); >> if (fd == -1) >> - errx(errno, "Cannot open %s", argv[optind]); >> + err(errno, "Cannot open %s", argv[optind]); >> >> nbd_disconnect(fd); >> >> @@ -333,23 +333,29 @@ int main(int argc, char **argv) >> if (bs == NULL) >> return 1; >> >> - if (bdrv_open(bs, argv[optind], flags) < 0) >> - return 1; >> + if ((ret = bdrv_open(bs, argv[optind], flags)) < 0) { >> + errno = -ret; >> + err(errno, "Failed to bdrv_open '%s'", argv[optind]); >> + } >> >> fd_size = bs->total_sectors * 512; >> >> if (partition != -1 && >> find_partition(bs, partition, &dev_offset, &fd_size)) >> - errx(errno, "Could not find partition %d", partition); >> + err(errno, "Could not find partition %d", partition); >> >> if (device) { >> pid_t pid; >> int sock; >> >> + /* want to fail before daemonizing */ >> + if (access(device, R_OK|W_OK) == -1) >> + err(errno, "Could not access '%s'", device); >> + >> if (!verbose) { >> /* detach client and server */ >> if (daemon(0, 0) == -1) { >> - errx(errno, "Failed to daemonize"); >> + err(errno, "Failed to daemonize"); >> } >> } >> >> > > -- > mailto:av1...@comtv.ru >
[Qemu-devel] [PATCH 2/2] qemu-nbd: Improve error reporting
- use err(3) instead of errx(3) if errno is available to report why failed - let fail prior to daemon(3) if opening a nbd file is likely to fail after daemonizing to avoid silent failure exit --- qemu-nbd.c | 16 +++- 1 files changed, 11 insertions(+), 5 deletions(-) diff --git a/qemu-nbd.c b/qemu-nbd.c index b89c361..5f10ff0 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -316,7 +316,7 @@ int main(int argc, char **argv) if (disconnect) { fd = open(argv[optind], O_RDWR); if (fd == -1) -errx(errno, "Cannot open %s", argv[optind]); +err(errno, "Cannot open %s", argv[optind]); nbd_disconnect(fd); @@ -333,23 +333,29 @@ int main(int argc, char **argv) if (bs == NULL) return 1; -if (bdrv_open(bs, argv[optind], flags) < 0) -return 1; +if ((ret = bdrv_open(bs, argv[optind], flags)) < 0) { +errno = -ret; +err(errno, "Failed to bdrv_open '%s'", argv[optind]); +} fd_size = bs->total_sectors * 512; if (partition != -1 && find_partition(bs, partition, &dev_offset, &fd_size)) -errx(errno, "Could not find partition %d", partition); +err(errno, "Could not find partition %d", partition); if (device) { pid_t pid; int sock; +/* want to fail before daemonizing */ +if (access(device, R_OK|W_OK) == -1) +err(errno, "Could not access '%s'", device); + if (!verbose) { /* detach client and server */ if (daemon(0, 0) == -1) { -errx(errno, "Failed to daemonize"); +err(errno, "Failed to daemonize"); } } -- 1.6.5.2
[Qemu-devel] [PATCH 1/2] qemu-nbd: Fix return value handling of bdrv_open
bdrv_open may return -errno so we have to check if the return value is '< 0', not '== -1'. --- qemu-nbd.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/qemu-nbd.c b/qemu-nbd.c index a393583..b89c361 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -333,7 +333,7 @@ int main(int argc, char **argv) if (bs == NULL) return 1; -if (bdrv_open(bs, argv[optind], flags) == -1) +if (bdrv_open(bs, argv[optind], flags) < 0) return 1; fd_size = bs->total_sectors * 512; -- 1.6.5.2
[Qemu-devel] [PATCH] qemu-nbd: Fix wrong description in qemu-nbd.texi
-c option needs argument but it's missing now. This patch fixes it. Signed-off-by: Ryota Ozaki --- qemu-nbd.texi |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/qemu-nbd.texi b/qemu-nbd.texi index ef13b34..44996cc 100644 --- a/qemu-nbd.texi +++ b/qemu-nbd.texi @@ -30,8 +30,8 @@ Export Qemu disk image using NBD protocol. use snapshot file @item -n, --nocache disable host cache -...@item -c, --connect - connect FILE to NBD device DEV +...@item -c, --conne...@var{dev} + connect @var{filename} to NBD device @var{dev} @item -d, --disconnect disconnect the specified device @item -e, --shar...@var{num} -- 1.6.5.2