On Tue, 2020-09-08 at 16:27 +0100, Stefan Hajnoczi wrote: > On Mon, Aug 31, 2020 at 06:01:24PM +0300, Maxim Levitsky wrote: > > Currently scsi_target_emulate_report_luns iterates > > over child devices list twice, and there is guarantee, that > > it will not be changed meanwhile. > > > > This reason for two loops is that it needs to know how much memory > > to allocate. > > > > Avoid this by iterating once, and allocating the memory for the output > > dynamically with reserving enought memory so that in practice it won't > > be reallocated often. > > > > Bugzilla for reference: https://bugzilla.redhat.com/show_bug.cgi?id=1866707 > > "Buglink:" is the tag name documented in > https://wiki.qemu.org/Contribute/SubmitAPatch#Write_a_meaningful_commit_message Noted > > > static bool scsi_target_emulate_report_luns(SCSITargetReq *r) > > { > > BusChild *kid; > > - int i, len, n; > > int channel, id; > > - bool found_lun0; > > + uint8_t tmp[8] = {0}; > > + int len = 0; > > + > > + /* reserve space for 63 LUNs*/ > > + GByteArray *buf = g_byte_array_sized_new(512); > > > > if (r->req.cmd.xfer < 16) { > > return false; > > buf is leaked. Oops, will fix > > > @@ -460,46 +466,36 @@ static bool > > scsi_target_emulate_report_luns(SCSITargetReq *r) > > } > > channel = r->req.dev->channel; > > id = r->req.dev->id; > > - found_lun0 = false; > > - n = 0; > > > > - rcu_read_lock(); > > > > - QTAILQ_FOREACH_RCU(kid, &r->req.bus->qbus.children, sibling) { > > - DeviceState *qdev = kid->child; > > - SCSIDevice *dev = SCSI_DEVICE(qdev); > > + /* add size (will be updated later to correct value */ > > + g_byte_array_append(buf, tmp, 8); > > + len += 8; > > Can g_byte_array_size() be used instead of keeping a len local variable? Glib don't seem to have this function, I checked the docs. Its seems that they want to convert it to GBytes which is basically immutible verion of GByteArray and it does have g_bytes_get_size. I decided that a local variable while ugly is still better that this.
I haven't wrote much code that uses Glib, so I might have missed something though. I had read this reference: https://developer.gnome.org/glib/stable/glib-Byte-Arrays.html > > Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com> Best regards, Maxim Levitsky
