From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40844) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XW1c6-0006MS-7D for qemu-devel@nongnu.org; Mon, 22 Sep 2014 07:17:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XW1c0-0000qN-2f for qemu-devel@nongnu.org; Mon, 22 Sep 2014 07:17:02 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41003) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XW1bz-0000ls-Rs for qemu-devel@nongnu.org; Mon, 22 Sep 2014 07:16:56 -0400 Date: Mon, 22 Sep 2014 14:20:09 +0300 From: "Michael S. Tsirkin" Message-ID: <20140922112009.GC14882@redhat.com> References: <1411082250-27776-1-git-send-email-drjones@redhat.com> <541BDCA0.6060102@huawei.com> <541BEC93.1030607@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <541BEC93.1030607@huawei.com> Subject: Re: [Qemu-devel] [PATCH v2] ivshmem: use error_report List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: zhanghailiang Cc: Andrew Jones , qemu-devel@nongnu.org, afaerber@suse.de On Fri, Sep 19, 2014 at 04:42:59PM +0800, zhanghailiang wrote: > On 2014/9/19 15:34, zhanghailiang wrote: > >On 2014/9/19 7:17, Andrew Jones wrote: > >>Replace all the fprintf(stderr, ...) calls with error_report. > >>Also make sure exit() consistently uses the error code 1. A few calls > >>used -1. > >> > >>Signed-off-by: Andrew Jones > >>--- > >> hw/misc/ivshmem.c | 39 +++++++++++++++++++-------------------- > >> 1 file changed, 19 insertions(+), 20 deletions(-) > >> > >>diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c > >>index bf585b7691998..b3983296f58fa 100644 > >>--- a/hw/misc/ivshmem.c > >>+++ b/hw/misc/ivshmem.c > >>@@ -300,8 +300,8 @@ static CharDriverState* create_eventfd_chr_device(void * opaque, EventNotifier * > >> chr = qemu_chr_open_eventfd(eventfd); > >> > >> if (chr == NULL) { > >>- fprintf(stderr, "creating eventfd for eventfd %d failed\n", eventfd); > >>- exit(-1); > >>+ error_report("creating eventfd for eventfd %d failed", eventfd); > >>+ exit(1); > >> } > >> qemu_chr_fe_claim_no_fail(chr); > >> > >>@@ -328,15 +328,14 @@ static int check_shm_size(IVShmemState *s, int fd) { > >> struct stat buf; > >> > >> if (fstat(fd, &buf) < 0) { > >>- fprintf(stderr, "ivshmem: exiting: fstat on fd %d failed: %s\n", > >>+ error_report("exiting: fstat on fd %d failed: %s", > >> fd, strerror(errno)); > > > >The indentation looks weird, better to fix it.;) > >More of the same elsewhere. > > > > Er, actually, maybe for print like function, it is no need to indent like other function. > So just ignore this comment, Sorry.;) No, I agree with the original comment. Please indent continuation lines to the right of the opening (, here and elsewhere. > >> return -1; > >> } > >> > >> if (s->ivshmem_size > buf.st_size) { > >>- fprintf(stderr, > >>- "IVSHMEM ERROR: Requested memory size greater" > >>- " than shared object size (%" PRIu64 " > %" PRIu64")\n", > >>+ error_report("Requested memory size greater" > >>+ " than shared object size (%" PRIu64 " > %" PRIu64")", > >> s->ivshmem_size, (uint64_t)buf.st_size); > >> return -1; > >> } else { > >>@@ -510,7 +509,7 @@ static void ivshmem_read(void *opaque, const uint8_t *buf, int size) > >> incoming_fd = dup(tmp_fd); > >> > >> if (incoming_fd == -1) { > >>- fprintf(stderr, "could not allocate file descriptor %s\n", > >>+ error_report("could not allocate file descriptor %s", > >> strerror(errno)); > >> close(tmp_fd); > >> return; > >>@@ -524,7 +523,7 @@ static void ivshmem_read(void *opaque, const uint8_t *buf, int size) > >> s->max_peer = 0; > >> > >> if (check_shm_size(s, incoming_fd) == -1) { > >>- exit(-1); > >>+ exit(1); > >> } > >> > >> /* mmap the region and map into the BAR2 */ > >>@@ -618,13 +617,13 @@ static uint64_t ivshmem_get_size(IVShmemState * s) { > >> value <<= 30; > >> break; > >> default: > >>- fprintf(stderr, "qemu: invalid ram size: %s\n", s->sizearg); > >>+ error_report("invalid ram size: %s", s->sizearg); > >> exit(1); > >> } > >> > >> /* BARs must be a power of 2 */ > >> if (!is_power_of_two(value)) { > >>- fprintf(stderr, "ivshmem: size must be power of 2\n"); > >>+ error_report("size must be power of 2"); > >> exit(1); > >> } > >> > >>@@ -676,7 +675,7 @@ static int ivshmem_load(QEMUFile* f, void *opaque, int version_id) > >> } > >> > >> if (proxy->role_val == IVSHMEM_PEER) { > >>- fprintf(stderr, "ivshmem: 'peer' devices are not migratable\n"); > >>+ error_report("'peer' devices are not migratable"); > >> return -EINVAL; > >> } > >> > >>@@ -722,7 +721,7 @@ static int pci_ivshmem_init(PCIDevice *dev) > >> /* IRQFD requires MSI */ > >> if (ivshmem_has_feature(s, IVSHMEM_IOEVENTFD) && > >> !ivshmem_has_feature(s, IVSHMEM_MSI)) { > >>- fprintf(stderr, "ivshmem: ioeventfd/irqfd requires MSI\n"); > >>+ error_report("ioeventfd/irqfd requires MSI"); > >> exit(1); > >> } > >> > >>@@ -733,7 +732,7 @@ static int pci_ivshmem_init(PCIDevice *dev) > >> } else if (strncmp(s->role, "master", 7) == 0) { > >> s->role_val = IVSHMEM_MASTER; > >> } else { > >>- fprintf(stderr, "ivshmem: 'role' must be 'peer' or 'master'\n"); > >>+ error_report("'role' must be 'peer' or 'master'"); > >> exit(1); > >> } > >> } else { > >>@@ -773,8 +772,8 @@ static int pci_ivshmem_init(PCIDevice *dev) > >> * to the ivshmem server to receive the memory region */ > >> > >> if (s->shmobj != NULL) { > >>- fprintf(stderr, "WARNING: do not specify both 'chardev' " > >>- "and 'shm' with ivshmem\n"); > >>+ error_report("WARNING: do not specify both 'chardev' " > >>+ "and 'shm' with ivshmem"); > >> } > >> > >> IVSHMEM_DPRINTF("using shared memory server (socket = %s)\n", > >>@@ -802,7 +801,7 @@ static int pci_ivshmem_init(PCIDevice *dev) > >> int fd; > >> > >> if (s->shmobj == NULL) { > >>- fprintf(stderr, "Must specify 'chardev' or 'shm' to ivshmem\n"); > >>+ error_report("Must specify 'chardev' or 'shm' to ivshmem"); > >> exit(1); > >> } > >> > >>@@ -814,18 +813,18 @@ static int pci_ivshmem_init(PCIDevice *dev) > >> S_IRWXU|S_IRWXG|S_IRWXO)) > 0) { > >> /* truncate file to length PCI device's memory */ > >> if (ftruncate(fd, s->ivshmem_size) != 0) { > >>- fprintf(stderr, "ivshmem: could not truncate shared file\n"); > >>+ error_report("could not truncate shared file"); > >> } > >> > >> } else if ((fd = shm_open(s->shmobj, O_CREAT|O_RDWR, > >> S_IRWXU|S_IRWXG|S_IRWXO)) < 0) { > >>- fprintf(stderr, "ivshmem: could not open shared file\n"); > >>- exit(-1); > >>+ error_report("could not open shared file"); > >>+ exit(1); > >> > >> } > >> > >> if (check_shm_size(s, fd) == -1) { > >>- exit(-1); > >>+ exit(1); > >> } > >> > >> create_shared_memory_BAR(s, fd); > >> > > > > > > > > > >. > > >