From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43466) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Za7IQ-0004jc-8S for qemu-devel@nongnu.org; Thu, 10 Sep 2015 15:14:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Za7IM-0005nf-Vn for qemu-devel@nongnu.org; Thu, 10 Sep 2015 15:14:10 -0400 Received: from mx2.parallels.com ([199.115.105.18]:39857) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Za7IM-0005nP-Qa for qemu-devel@nongnu.org; Thu, 10 Sep 2015 15:14:06 -0400 References: <1441699228-25767-1-git-send-email-den@openvz.org> <1441699228-25767-6-git-send-email-den@openvz.org> <55EF0AB4.6090906@redhat.com> From: "Denis V. Lunev" Message-ID: <55F1D673.2060905@openvz.org> Date: Thu, 10 Sep 2015 22:13:55 +0300 MIME-Version: 1.0 In-Reply-To: <55EF0AB4.6090906@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 5/5] disk_deadlines: add info disk-deadlines option List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Kevin Wolf , Markus Armbruster , qemu-devel@nongnu.org, Raushaniya Maksudova , Luiz Capitulino , Stefan Hajnoczi On 09/08/2015 07:20 PM, Eric Blake wrote: > On 09/08/2015 02:00 AM, Denis V. Lunev wrote: >> From: Raushaniya Maksudova >> >> This patch adds "info disk-deadlines" qemu-monitor option that prints >> dump of all disk requests which caused a disk deadline in Guest OS >> from the very start of Virtual Machine: >> >> disk_id type size total_time start_time >> .-------------------------------------------------------- >> ide0-hd1 FLUSH 0b 46.403s 22232930059574ns >> ide0-hd1 FLUSH 0b 57.591s 22451499241285ns >> ide0-hd1 FLUSH 0b 103.482s 22574100547397ns >> >> Signed-off-by: Raushaniya Maksudova >> Signed-off-by: Denis V. Lunev >> CC: Stefan Hajnoczi >> CC: Kevin Wolf >> CC: Markus Armbruster >> CC: Luiz Capitulino >> --- > qapi interface review only: > > >> +++ b/qapi-schema.json >> @@ -3808,3 +3808,36 @@ >> >> # Rocker ethernet network switch >> { 'include': 'qapi/rocker.json' } >> + >> +## @DiskDeadlinesInfo >> +# >> +# Contains info about late requests which caused VM stopping >> +# >> +# @disk-id: name of disk (unique for each disk) > Mark this with '#optional', and maybe describe why it would be missing. > Does this correspond to the BDS node name where the deadline expired, > in which case 'node' might be a nicer name than 'disk-id'? as far as I could understand the code this is not BDS node name. This name is bound to a name of a hardware device under which we could have several block drivers at the moment. There is no query by this name at the moment and the identifier collected is good enough for us for a while to understand and debug the code. Exact name is to be defined though. Originally I would like to bind it to device name deadlines are attached to and as it would not be good to calculate them in each BDS. Anyway, exact meaning of this 'id' will be defined when we will decide on proper attach point, whether it will be generic block code or filter driver or whatever also. Does it sound good for you? All your suggestions are welcome. >> +# >> +# @type: type of request could be READ, WRITE or FLUSH > Likewise for using #optional. Please make this an enum type, not an > open-coded string. ok >> +# >> +# @size: size in bytes > of the failed request? Should you also mention which offset the failed > request started at? I'll add this. This info is not accessible in stats and that's why it was not added. If the code will be a part of the block layer or will be in the driver filter that would be not a problem. >> +# >> +# @total-time-ns: total time of request execution >> +# >> +# @start-time-ns: indicates the start of request execution >> +# >> +# Since: 2.5 >> +## >> +{ 'struct': 'DiskDeadlinesInfo', >> + 'data' : { '*disk-id': 'str', >> + '*type': 'str', >> + 'size': 'uint64', >> + 'total-time-ns': 'uint64', >> + 'start-time-ns': 'uint64' } } >> +## >> +# @query-disk-deadlines: >> +# >> +# Returns information about last late disk requests. >> +# >> +# Returns: a list of @DiskDeadlinesInfo >> +# >> +# Since: 2.5 >> +## >> +{ 'command': 'query-disk-deadlines', 'returns': ['DiskDeadlinesInfo'] } > Should it be possible to filter to deadlines missed for a specific node, > by having an arguments with an optional node name? ok, this seems quite reasonable. > Should any of the existing query-block or similar commands be modified > to make it obvious that there are missed deadline stats, and that it > would be useful to call query-disk-deadlines to learn more about them? > What do you think if we will also provide the list of pending requests using similar API? Would it be useful for others? Den