| Message ID | 1452599056-27357-2-git-send-email-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, 12 Jan 2016 11:43:55 +0000 "Daniel P. Berrange" <berrange@redhat.com> wrote: > The s390 skeys monitor command needs to write out a plain text > file. Currently it is using the QEMUFile class for this. There > is no real benefit to this, and the downside is that it needs to > snprintf via an intermediate buffer. Switching to regular FILE > objects simplifies the code. > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > --- > hw/s390x/s390-skeys.c | 19 +++++++------------ > 1 file changed, 7 insertions(+), 12 deletions(-) > @@ -124,7 +119,7 @@ void qmp_dump_skeys(const char *filename, Error **errp) > return; > } > > - f = qemu_fopen(filename, "wb"); > + f = fopen(filename, "wb"); > if (!f) { > error_setg_file_open(errp, errno, filename); > return; Hmm... https://marc.info/?l=qemu-devel&m=143740278908660&w=2 We'd lose the benefits from qemu_fopen() here again, or am I missing something?
On Tue, Jan 12, 2016 at 12:58:10PM +0100, Cornelia Huck wrote: > On Tue, 12 Jan 2016 11:43:55 +0000 > "Daniel P. Berrange" <berrange@redhat.com> wrote: > > > The s390 skeys monitor command needs to write out a plain text > > file. Currently it is using the QEMUFile class for this. There > > is no real benefit to this, and the downside is that it needs to > > snprintf via an intermediate buffer. Switching to regular FILE > > objects simplifies the code. > > > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > > --- > > hw/s390x/s390-skeys.c | 19 +++++++------------ > > 1 file changed, 7 insertions(+), 12 deletions(-) > > > @@ -124,7 +119,7 @@ void qmp_dump_skeys(const char *filename, Error **errp) > > return; > > } > > > > - f = qemu_fopen(filename, "wb"); > > + f = fopen(filename, "wb"); > > if (!f) { > > error_setg_file_open(errp, errno, filename); > > return; > > Hmm... > > https://marc.info/?l=qemu-devel&m=143740278908660&w=2 > > We'd lose the benefits from qemu_fopen() here again, or am I missing > something? Oh, that message shouldn't really have recommended qemu_fopen() - it only needs qemu_open() to get the fd passing support. I'll change this to use qemu_open() + fdopen() instead, so we still support FD passing while using normal stdio. Regards, Daniel
On Tue, 12 Jan 2016 12:01:21 +0000 "Daniel P. Berrange" <berrange@redhat.com> wrote: > On Tue, Jan 12, 2016 at 12:58:10PM +0100, Cornelia Huck wrote: > > On Tue, 12 Jan 2016 11:43:55 +0000 > > "Daniel P. Berrange" <berrange@redhat.com> wrote: > > > > > The s390 skeys monitor command needs to write out a plain text > > > file. Currently it is using the QEMUFile class for this. There > > > is no real benefit to this, and the downside is that it needs to > > > snprintf via an intermediate buffer. Switching to regular FILE > > > objects simplifies the code. > > > > > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > > > --- > > > hw/s390x/s390-skeys.c | 19 +++++++------------ > > > 1 file changed, 7 insertions(+), 12 deletions(-) > > > > > @@ -124,7 +119,7 @@ void qmp_dump_skeys(const char *filename, Error **errp) > > > return; > > > } > > > > > > - f = qemu_fopen(filename, "wb"); > > > + f = fopen(filename, "wb"); > > > if (!f) { > > > error_setg_file_open(errp, errno, filename); > > > return; > > > > Hmm... > > > > https://marc.info/?l=qemu-devel&m=143740278908660&w=2 > > > > We'd lose the benefits from qemu_fopen() here again, or am I missing > > something? > > Oh, that message shouldn't really have recommended qemu_fopen() - it only > needs qemu_open() to get the fd passing support. I'll change this to use > qemu_open() + fdopen() instead, so we still support FD passing while > using normal stdio. Sounds good.
* Daniel P. Berrange (berrange@redhat.com) wrote: > The s390 skeys monitor command needs to write out a plain text > file. Currently it is using the QEMUFile class for this. There > is no real benefit to this, and the downside is that it needs to > snprintf via an intermediate buffer. Switching to regular FILE > objects simplifies the code. > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Dave > --- > hw/s390x/s390-skeys.c | 19 +++++++------------ > 1 file changed, 7 insertions(+), 12 deletions(-) > > diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c > index 539ef6d..637ccf5 100644 > --- a/hw/s390x/s390-skeys.c > +++ b/hw/s390x/s390-skeys.c > @@ -45,15 +45,11 @@ void s390_skeys_init(void) > qdev_init_nofail(DEVICE(obj)); > } > > -static void write_keys(QEMUFile *f, uint8_t *keys, uint64_t startgfn, > +static void write_keys(FILE *f, uint8_t *keys, uint64_t startgfn, > uint64_t count, Error **errp) > { > uint64_t curpage = startgfn; > uint64_t maxpage = curpage + count - 1; > - const char *fmt = "page=%03" PRIx64 ": key(%d) => ACC=%X, FP=%d, REF=%d," > - " ch=%d, reserved=%d\n"; > - char buf[128]; > - int len; > > for (; curpage <= maxpage; curpage++) { > uint8_t acc = (*keys & 0xF0) >> 4; > @@ -62,10 +58,9 @@ static void write_keys(QEMUFile *f, uint8_t *keys, uint64_t startgfn, > int ch = (*keys & 0x02); > int res = (*keys & 0x01); > > - len = snprintf(buf, sizeof(buf), fmt, curpage, > - *keys, acc, fp, ref, ch, res); > - assert(len < sizeof(buf)); > - qemu_put_buffer(f, (uint8_t *)buf, len); > + fprintf(f, "page=%03" PRIx64 ": key(%d) => ACC=%X, FP=%d, REF=%d," > + " ch=%d, reserved=%d\n", > + curpage, *keys, acc, fp, ref, ch, res); > keys++; > } > } > @@ -115,7 +110,7 @@ void qmp_dump_skeys(const char *filename, Error **errp) > vaddr cur_gfn = 0; > uint8_t *buf; > int ret; > - QEMUFile *f; > + FILE *f; > > /* Quick check to see if guest is using storage keys*/ > if (!skeyclass->skeys_enabled(ss)) { > @@ -124,7 +119,7 @@ void qmp_dump_skeys(const char *filename, Error **errp) > return; > } > > - f = qemu_fopen(filename, "wb"); > + f = fopen(filename, "wb"); > if (!f) { > error_setg_file_open(errp, errno, filename); > return; > @@ -161,7 +156,7 @@ out_free: > error_propagate(errp, lerr); > g_free(buf); > out: > - qemu_fclose(f); > + fclose(f); > } > > static void qemu_s390_skeys_init(Object *obj) > -- > 2.5.0 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c index 539ef6d..637ccf5 100644 --- a/hw/s390x/s390-skeys.c +++ b/hw/s390x/s390-skeys.c @@ -45,15 +45,11 @@ void s390_skeys_init(void) qdev_init_nofail(DEVICE(obj)); } -static void write_keys(QEMUFile *f, uint8_t *keys, uint64_t startgfn, +static void write_keys(FILE *f, uint8_t *keys, uint64_t startgfn, uint64_t count, Error **errp) { uint64_t curpage = startgfn; uint64_t maxpage = curpage + count - 1; - const char *fmt = "page=%03" PRIx64 ": key(%d) => ACC=%X, FP=%d, REF=%d," - " ch=%d, reserved=%d\n"; - char buf[128]; - int len; for (; curpage <= maxpage; curpage++) { uint8_t acc = (*keys & 0xF0) >> 4; @@ -62,10 +58,9 @@ static void write_keys(QEMUFile *f, uint8_t *keys, uint64_t startgfn, int ch = (*keys & 0x02); int res = (*keys & 0x01); - len = snprintf(buf, sizeof(buf), fmt, curpage, - *keys, acc, fp, ref, ch, res); - assert(len < sizeof(buf)); - qemu_put_buffer(f, (uint8_t *)buf, len); + fprintf(f, "page=%03" PRIx64 ": key(%d) => ACC=%X, FP=%d, REF=%d," + " ch=%d, reserved=%d\n", + curpage, *keys, acc, fp, ref, ch, res); keys++; } } @@ -115,7 +110,7 @@ void qmp_dump_skeys(const char *filename, Error **errp) vaddr cur_gfn = 0; uint8_t *buf; int ret; - QEMUFile *f; + FILE *f; /* Quick check to see if guest is using storage keys*/ if (!skeyclass->skeys_enabled(ss)) { @@ -124,7 +119,7 @@ void qmp_dump_skeys(const char *filename, Error **errp) return; } - f = qemu_fopen(filename, "wb"); + f = fopen(filename, "wb"); if (!f) { error_setg_file_open(errp, errno, filename); return; @@ -161,7 +156,7 @@ out_free: error_propagate(errp, lerr); g_free(buf); out: - qemu_fclose(f); + fclose(f); } static void qemu_s390_skeys_init(Object *obj)
The s390 skeys monitor command needs to write out a plain text file. Currently it is using the QEMUFile class for this. There is no real benefit to this, and the downside is that it needs to snprintf via an intermediate buffer. Switching to regular FILE objects simplifies the code. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- hw/s390x/s390-skeys.c | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-)